Add tool permission system: approval prompts, policy, plan mode

index 7e71441..f39fcd4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -102,6 +102,17 @@ feeds results back. Neither the loop nor ACP/TUI surfaces depend on a concrete b official server (`<cloud>/mcp`, default `https://sigit.si/api/v1/mcp`) is baked in and authed with the cloud session token; extra servers live in `mcp.toml` (global `$SIGIT_CONFIG_DIR/mcp.toml` and project-local `.sigit/mcp.toml`). stdio transport is not supported. +- **`src/permissions.rs`** — tool permission policy. Every tool call passes through + `decision_for` before executing: read-only tools always run; mutating tools (and all + `mcp__*`/unknown tools) are governed by, in order: per-session plan mode (`/plan` — deny all + mutating tools with a present-a-plan message), session "always allow" grants, per-tool + overrides and the default mode from `[permissions]` in `settings.toml` (`allow`/`ask`/`deny`, + default `ask`; `SIGIT_PERMISSIONS` env overrides the default). On `ask`, the ACP path sends + `session/request_permission` (allow once / allow for session / deny) and the TUI pauses the + inference task on a y/a/n prompt. Note: ACP turn-affecting handlers run in `cx.spawn`ed tasks + serialized by `SiGitAgent::turn_lock` so the dispatch loop can route the client's permission + answer mid-turn — don't move them back inline, and don't await client requests from inline + handlers (deadlock). - **`src/instructions.rs`** — project instruction files, the always-on counterpart to skills. Reads `AGENTS.md` (the cross-tool [agents.md](https://agents.md) standard) and `CLAUDE.md`, walking from the session cwd up to the repo root (nearest ancestor with `.git`, never above it), @@ -120,8 +131,8 @@ feeds results back. Neither the loop nor ACP/TUI surfaces depend on a concrete b - **`src/models.rs`** — model-picker types shared across platforms. Slash commands (`/help`, `/models`, `/skills`, `/mcp`, `/login`, `/logout`, `/whoami`, `/reload`, -`/clear`, `/status`) are advertised via `advertise_commands` in `main.rs` and handled in both the -TUI and ACP sessions. +`/plan`, `/permissions`, `/clear`, `/status`) are advertised via `advertise_commands` in `main.rs` +and handled in both the TUI and ACP sessions. ## Model cache (macOS) @@ -142,7 +153,9 @@ verbosity with `RUST_LOG`. `OPENAI_BASE_URL` / `OPENAI_API_KEY` (provider override), `SIGIT_API_URL` (account API base, default `https://sigit.si`), `SIGIT_CLOUD_URL`, `SIGIT_CONFIG_DIR` (default `~/.config/sigit`), `SIGIT_MODEL`, `SIGIT_MCP` (`off` disables MCP), `SIGIT_MCP_OFFICIAL` (`off` drops the baked-in -server), `HF_HOME` / `HF_HUB_CACHE`, `RUST_LOG`. +server), `SIGIT_PERMISSIONS` (`allow`/`ask`/`deny` — overrides the default permission mode for +mutating tools; the escape hatch for clients without permission-request support), +`HF_HOME` / `HF_HUB_CACHE`, `RUST_LOG`. ## Releasing

index b0a7a01..2038ebc 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -88,7 +88,7 @@ mod tui { text::{Line, Span}, widgets::{Block, Borders, Clear, Paragraph, Wrap}, }; - use tokio::sync::mpsc; + use tokio::sync::{mpsc, oneshot}; use tokio::time::{Duration, Instant, interval}; // ── Message types ───────────────────────────────────────────────────────── @@ -157,6 +157,23 @@ mod tui { /// a complete (non-streamed) assistant reply Response(String), Error(String), + /// the inference task wants to run a mutating tool and is paused on + /// `reply`; the user answers with y (once) / a (session) / n (deny) + ApprovalRequest { + tool: String, + reply: oneshot::Sender<ApprovalChoice>, + }, + } + + /// The user's answer to a tool-approval prompt. Dropping the reply channel + /// (quit, cancel) counts as a denial on the inference side. + enum ApprovalChoice { + /// run this one call + Once, + /// run it and stop asking for this tool for the rest of the session + Session, + /// skip the call; the model gets an explanatory tool result + Deny, } enum ModelLoadUpdate { @@ -175,6 +192,9 @@ mod tui { stream_buf: String, inference_rx: Option<mpsc::Receiver<InferenceUpdate>>, model_load_rx: Option<mpsc::Receiver<ModelLoadUpdate>>, + /// a tool call waiting on the user's y/a/n answer; the inference task is + /// paused on the other end of the channel + pending_approval: Option<(String, oneshot::Sender<ApprovalChoice>)>, thinking: bool, thinking_tick: u8, quit: bool, @@ -270,6 +290,7 @@ mod tui { stream_buf: String::new(), inference_rx: None, model_load_rx: None, + pending_approval: None, thinking: false, thinking_tick: 0, quit: false, @@ -342,6 +363,9 @@ mod tui { fn stop_thinking(&mut self) { self.thinking = false; self.inference_rx = None; + // Dropping a pending reply channel reads as a denial on the + // inference side, so a cancelled turn can't leave a tool waiting. + self.pending_approval = None; } fn tick_thinking(&mut self) { @@ -746,6 +770,11 @@ mod tui { Login(Option<String>), Logout, Whoami, + /// Toggle plan mode (research only; mutating tools are denied with a + /// prompt to present a plan). `Some(true/false)` sets it, `None` flips it. + Plan(Option<bool>), + /// Show the effective permission policy for this session. + Permissions, Exit, Unknown(String), } @@ -770,6 +799,8 @@ mod tui { "/login" => SlashCommand::Login(arg.map(str::to_string)), "/logout" => SlashCommand::Logout, "/whoami" => SlashCommand::Whoami, + "/plan" => SlashCommand::Plan(parse_on_off(arg)), + "/permissions" => SlashCommand::Permissions, "/exit" | "/quit" | "/q" => SlashCommand::Exit, other => SlashCommand::Unknown(other.to_string()), }) @@ -1139,7 +1170,12 @@ mod tui { Span::styled(" quit", Style::default().fg(Color::DarkGray)), ]; - if app.thinking || app.switching_model || app.is_streaming() { + if let Some((tool, _)) = &app.pending_approval { + spans.push(Span::styled( + format!(" allow {tool}? [y]es · [a]lways · [n]o"), + Style::default().fg(Color::Yellow), + )); + } else if app.thinking || app.switching_model || app.is_streaming() { spans.push(Span::styled( " (busy — Ctrl+C to cancel)", Style::default().fg(Color::Yellow), @@ -1367,6 +1403,8 @@ mod tui { /login E P — sign in to siGit Code Cloud\n\ /logout — sign out\n\ /whoami — show the signed-in account\n\ + /plan [on|off] — plan mode: research only, no edits or commands\n\ + /permissions — show the tool permission policy\n\ /clear — wipe conversation history\n\ /status — show engine status\n\ /exit — quit chat", @@ -1375,10 +1413,29 @@ mod tui { SlashCommand::Clear => { let cleared = engine.clear_history().await; app.messages.clear(); + crate::permissions::reset_session(crate::permissions::TUI_SESSION); app.messages.push(ChatMessage::system(format!( "Cleared {cleared} turn(s). History is empty.", ))); } + SlashCommand::Plan(value) => { + use crate::permissions::{self, TUI_SESSION}; + let enabled = value.unwrap_or_else(|| !permissions::plan_mode(TUI_SESSION)); + permissions::set_plan_mode(TUI_SESSION, enabled); + app.messages.push(ChatMessage::system(if enabled { + "Plan mode ON — research with read-only tools only; edits and commands \ + are blocked until /plan off." + } else { + "Plan mode OFF — tools may execute again (subject to the permission \ + policy)." + })); + } + SlashCommand::Permissions => { + app.messages + .push(ChatMessage::system(crate::permissions::describe( + crate::permissions::TUI_SESSION, + ))); + } SlashCommand::Status => { let info = engine.as_ref().info().await; let model = info.model_name.as_deref().unwrap_or("(none)"); @@ -1617,7 +1674,41 @@ mod tui { let _ = tx.send(InferenceUpdate::ToolUse(tc.name.clone())).await; - let output = crate::tools::execute_tool(&tc.name, &tc.arguments).await; + // Permission gate: read-only tools pass straight through; a + // mutating tool consults policy and may pause on the user's + // y/a/n answer (delivered over a oneshot from the event loop). + use crate::permissions::{self, Decision, TUI_SESSION}; + let output = match permissions::decision_for(TUI_SESSION, &tc.name) { + Decision::Allow => crate::tools::execute_tool(&tc.name, &tc.arguments).await, + Decision::Deny(reason) => { + log::info!(" ✗ {} denied by policy", tc.name); + reason + } + Decision::Ask => { + let (reply_tx, reply_rx) = oneshot::channel(); + let _ = tx + .send(InferenceUpdate::ApprovalRequest { + tool: tc.name.clone(), + reply: reply_tx, + }) + .await; + match reply_rx.await { + Ok(ApprovalChoice::Once) => { + crate::tools::execute_tool(&tc.name, &tc.arguments).await + } + Ok(ApprovalChoice::Session) => { + permissions::grant_for_session(TUI_SESSION, &tc.name); + crate::tools::execute_tool(&tc.name, &tc.arguments).await + } + // An explicit "no", or the UI dropped the channel + // (cancel/quit) — either way, do not run the tool. + Ok(ApprovalChoice::Deny) | Err(_) => { + log::info!(" ✗ {} denied by user", tc.name); + permissions::user_denial(&tc.name) + } + } + } + }; log::info!(" ← {} chars", output.len()); tool_results.push(ToolResult { @@ -1837,6 +1928,12 @@ mod tui { app.stop_thinking(); app.messages.push(ChatMessage::system(format!("error: {msg}"))); } + Some(InferenceUpdate::ApprovalRequest { tool, reply }) => { + app.messages.push(ChatMessage::system(format!( + "⚠ permission — allow {tool}? [y]es · [a]lways this session · [n]o" + ))); + app.pending_approval = Some((tool, reply)); + } None => { // task finished, possibly with no text to show app.finalize_stream(); @@ -1881,6 +1978,53 @@ mod tui { continue; } + // pending tool approval — y/a/n answer the prompt; the + // inference task is paused on the reply channel. Checked + // before the busy gate because the app *is* busy here. + if app.pending_approval.is_some() { + if key.kind == KeyEventKind::Press { + let ctrl = key.modifiers.contains(KeyModifiers::CONTROL); + let choice = if ctrl + && (key.code == KeyCode::Char('c') + || key.code == KeyCode::Char('d')) + { + // cancel the whole turn: denying is implicit + // in dropping the reply channel + app.pending_approval = None; + app.stop_thinking(); + app.messages.push(ChatMessage::system("(cancelled)")); + continue; + } else { + match key.code { + KeyCode::Char('y') | KeyCode::Char('Y') => { + Some(ApprovalChoice::Once) + } + KeyCode::Char('a') | KeyCode::Char('A') => { + Some(ApprovalChoice::Session) + } + KeyCode::Char('n') | KeyCode::Char('N') | KeyCode::Esc => { + Some(ApprovalChoice::Deny) + } + _ => None, + } + }; + if let Some(choice) = choice + && let Some((tool, reply)) = app.pending_approval.take() + { + let verdict = match &choice { + ApprovalChoice::Once => "allowed once", + ApprovalChoice::Session => "allowed for this session", + ApprovalChoice::Deny => "denied", + }; + app.messages.push(ChatMessage::system(format!( + "{tool}: {verdict}" + ))); + let _ = reply.send(choice); + } + } + continue; + } + // busy — only cancel keys work if app.is_busy() { if key.kind == KeyEventKind::Press {

index 77c2f35..2d719f3 100644 --- a/src/main.rs +++ b/src/main.rs @@ -35,6 +35,7 @@ mod credentials; mod instructions; mod mcp; mod models; +mod permissions; mod provider; mod settings; mod setup; @@ -62,12 +63,13 @@ use agent_client_protocol::schema::v1::{ AvailableCommand, AvailableCommandInput, AvailableCommandsUpdate, CancelNotification, ConfigOptionUpdate, ContentBlock, ContentChunk, EmbeddedResourceResource, ForkSessionRequest, ForkSessionResponse, Implementation, InitializeRequest, InitializeResponse, LoadSessionRequest, - LoadSessionResponse, Meta, NewSessionRequest, NewSessionResponse, PromptRequest, - PromptResponse, SessionCapabilities, SessionConfigOption, SessionConfigOptionCategory, - SessionConfigSelectOption, SessionConfigValueId, SessionForkCapabilities, SessionId, - SessionNotification, SessionUpdate, SetSessionConfigOptionRequest, - SetSessionConfigOptionResponse, StopReason, ToolCall, ToolCallStatus, ToolCallUpdate, - ToolCallUpdateFields, ToolKind, UnstructuredCommandInput, + LoadSessionResponse, Meta, NewSessionRequest, NewSessionResponse, PermissionOption, + PermissionOptionKind, PromptRequest, PromptResponse, RequestPermissionOutcome, + RequestPermissionRequest, SessionCapabilities, SessionConfigOption, + SessionConfigOptionCategory, SessionConfigSelectOption, SessionConfigValueId, + SessionForkCapabilities, SessionId, SessionNotification, SessionUpdate, + SetSessionConfigOptionRequest, SetSessionConfigOptionResponse, StopReason, ToolCall, + ToolCallStatus, ToolCallUpdate, ToolCallUpdateFields, ToolKind, UnstructuredCommandInput, }; use agent_client_protocol::{Agent, ByteStreams, Client, ConnectionTo, Responder}; use onde::inference::{ChatEngine, GgufModelConfig}; @@ -230,6 +232,33 @@ pub(crate) fn system_prompt_for_model(tool_calling: bool) -> &'static str { /// cap tool-call loops so a confused model can't spin forever const MAX_TOOL_ROUNDS: usize = 10; +/// Outcome of asking the client for permission to run one tool call. +enum PermissionVerdict { + /// Run the tool. + Approved, + /// Skip the tool; the string becomes its tool result so the model adapts. + Denied(String), + /// The client cancelled the turn while the request was pending; stop the + /// whole prompt with `StopReason::Cancelled` instead of burning rounds. + TurnCancelled, +} + +/// Display kind for the permission dialog, so editors can show a fitting icon. +fn tool_kind_for(tool_name: &str) -> ToolKind { + match tool_name { + "edit_file" | "multi_edit" | "create_file" | "create_directory" | "remember" => { + ToolKind::Edit + } + "delete_file" => ToolKind::Delete, + "run_command" => ToolKind::Execute, + "read_file" | "list_directory" => ToolKind::Read, + "search_files" | "glob" => ToolKind::Search, + "read_website" => ToolKind::Fetch, + "write_todos" => ToolKind::Think, + _ => ToolKind::Other, + } +} + /// Shown when a siGit Code Cloud tier is selected without a signed-in account. const CLOUD_LOGIN_PROMPT: &str = "siGit Code Cloud needs an account. Sign in with \ `/login <email> <password>` (or the Authenticate button), then pick the tier again. \ @@ -350,6 +379,12 @@ struct SiGitAgent { startup_model_name: String, /// for download-progress polling startup_model_id: String, + /// Serializes turn-affecting handlers (prompt, session lifecycle, config + /// changes). They run in `cx.spawn`ed tasks so the JSON-RPC dispatch loop + /// stays free to route client responses (e.g. permission answers) mid-turn; + /// this lock reproduces the strict ordering the dispatch loop used to give + /// them for free. + turn_lock: Arc<tokio::sync::Mutex<()>>, } impl SiGitAgent { @@ -375,6 +410,7 @@ impl SiGitAgent { startup_needs_download, startup_model_name, startup_model_id, + turn_lock: Arc::new(tokio::sync::Mutex::new(())), } } @@ -727,6 +763,12 @@ impl SiGitAgent { AvailableCommand::new("logout", "Sign out of siGit Code Cloud"), AvailableCommand::new("whoami", "Show the signed-in account"), AvailableCommand::new("reload", "Re-sync sign-in and model state"), + with_hint( + "plan", + "Plan mode: research only, no edits or commands", + "on|off (optional)", + ), + AvailableCommand::new("permissions", "Show the tool permission policy"), AvailableCommand::new("clear", "Wipe the conversation history"), AvailableCommand::new("status", "Show engine status"), ]; @@ -907,6 +949,10 @@ impl SiGitAgent { *guard = Some(args.cwd.clone()); } + // A reloaded session starts fresh: grants and plan mode from the + // previous life of this session id must not carry over. + permissions::reset_session(&args.session_id.to_string()); + // tool calls use relative paths, so we need to match the editor's cwd if args.cwd.is_dir() && let Err(err) = std::env::set_current_dir(&args.cwd) @@ -1256,7 +1302,35 @@ impl SiGitAgent { tc.arguments.chars().take(120).collect::<String>() ); - let output = tools::execute_tool(&tc.name, &tc.arguments).await; + // Permission gate: read-only tools pass straight through; a + // mutating tool consults policy and may ask the client. + let output = match permissions::decision_for(&session_id.to_string(), &tc.name) { + permissions::Decision::Allow => { + tools::execute_tool(&tc.name, &tc.arguments).await + } + permissions::Decision::Deny(reason) => { + log::info!(" ✗ {} denied by policy", tc.name); + reason + } + permissions::Decision::Ask => { + match self + .request_tool_permission(cx, &session_id, &tc.name, &tc.arguments) + .await + { + PermissionVerdict::Approved => { + tools::execute_tool(&tc.name, &tc.arguments).await + } + PermissionVerdict::Denied(reason) => { + log::info!(" ✗ {} denied by user", tc.name); + reason + } + PermissionVerdict::TurnCancelled => { + log::info!("prompt({}) cancelled at permission gate", session_id); + return Ok(PromptResponse::new(StopReason::Cancelled)); + } + } + } + }; log::info!(" ← {} chars", output.len()); @@ -1323,6 +1397,74 @@ impl SiGitAgent { Ok(PromptResponse::new(StopReason::EndTurn)) } + /// Ask the ACP client for permission to run one tool call. Presents + /// allow-once / allow-for-session / deny; an "always allow" choice is + /// recorded via [`permissions::grant_for_session`]. Only safe to call from + /// a spawned task (see the handler registration in `run_acp_server`): the + /// dispatch loop must be free to route the client's answer back to us. + async fn request_tool_permission( + &self, + cx: &ConnectionTo<Client>, + session_id: &SessionId, + tool_name: &str, + arguments: &str, + ) -> PermissionVerdict { + let args_preview: String = arguments.chars().take(120).collect(); + let title = if args_preview.is_empty() { + tool_name.to_string() + } else { + format!("{tool_name}({args_preview})") + }; + + let request = RequestPermissionRequest::new( + session_id.clone(), + ToolCallUpdate::new( + format!("perm-{}", uuid::Uuid::new_v4()), + ToolCallUpdateFields::new() + .title(title) + .kind(tool_kind_for(tool_name)) + .status(ToolCallStatus::Pending), + ), + vec![ + PermissionOption::new("allow_once", "Allow once", PermissionOptionKind::AllowOnce), + PermissionOption::new( + "allow_session", + "Allow for this session", + PermissionOptionKind::AllowAlways, + ), + PermissionOption::new("reject_once", "Deny", PermissionOptionKind::RejectOnce), + ], + ); + + match cx.send_request(request).block_task().await { + Ok(response) => match response.outcome { + RequestPermissionOutcome::Selected(selected) => { + match selected.option_id.0.as_ref() { + "allow_once" => PermissionVerdict::Approved, + "allow_session" => { + permissions::grant_for_session(&session_id.to_string(), tool_name); + PermissionVerdict::Approved + } + _ => PermissionVerdict::Denied(permissions::user_denial(tool_name)), + } + } + RequestPermissionOutcome::Cancelled => PermissionVerdict::TurnCancelled, + // The outcome enum is non_exhaustive; treat anything unknown as + // a denial rather than running a mutating tool unapproved. + _ => PermissionVerdict::Denied(permissions::user_denial(tool_name)), + }, + Err(error) => { + log::warn!("permission request for `{tool_name}` failed: {error}"); + PermissionVerdict::Denied(format!( + "`{tool_name}` was not executed: this client could not answer the \ + permission request ({error}). The user can pre-approve tools in \ + settings.toml under [permissions], or set SIGIT_PERMISSIONS=allow \ + for clients without permission support." + )) + } + } + } + async fn handle_cancel(&self, args: CancelNotification) -> agent_client_protocol::Result<()> { log::info!("cancel requested for session {}", args.session_id); Ok(()) @@ -1977,6 +2119,11 @@ enum SlashCommand { Whoami, /// Re-sync session state (auth, backend, picker) without a new session. Reload, + /// Toggle plan mode (read-only research; mutating tools denied with a + /// prompt to present a plan). `Some(true/false)` sets it, `None` flips it. + Plan(Option<bool>), + /// Show the effective permission policy for this session. + Permissions, Exit, Unknown(String), } @@ -2002,6 +2149,8 @@ fn parse_slash(input: &str) -> Option<SlashCommand> { "/logout" => SlashCommand::Logout, "/whoami" => SlashCommand::Whoami, "/reload" => SlashCommand::Reload, + "/plan" => SlashCommand::Plan(parse_on_off(argument)), + "/permissions" => SlashCommand::Permissions, "/exit" | "/quit" | "/q" => SlashCommand::Exit, other => SlashCommand::Unknown(other.to_string()), }) @@ -2110,6 +2259,8 @@ async fn exec_slash_acp( /logout - sign out\n\ /whoami - show the signed-in account\n\ /reload - re-sync sign-in and model state\n\ + /plan [on|off] - plan mode: research only, no edits or commands\n\ + /permissions - show the tool permission policy\n\ /clear - wipe conversation history\n\ /status - show engine status\n\ /exit - end this turn", @@ -2118,6 +2269,7 @@ async fn exec_slash_acp( } SlashCommand::Clear => { let cleared = agent.engine.clear_history().await; + permissions::reset_session(&session_id.to_string()); agent .send_assistant_message( cx, @@ -2126,6 +2278,23 @@ async fn exec_slash_acp( ) .ok(); } + SlashCommand::Plan(value) => { + let session_key = session_id.to_string(); + let enabled = value.unwrap_or_else(|| !permissions::plan_mode(&session_key)); + permissions::set_plan_mode(&session_key, enabled); + let message = if enabled { + "Plan mode ON — the agent researches with read-only tools and presents a \ + plan; edits and commands are blocked until /plan off." + } else { + "Plan mode OFF — the agent may execute tools again (subject to the \ + permission policy)." + }; + agent.send_assistant_message(cx, session_id, message).ok(); + } + SlashCommand::Permissions => { + let summary = permissions::describe(&session_id.to_string()); + agent.send_assistant_message(cx, session_id, summary).ok(); + } SlashCommand::Status => { let info = agent.engine.info().await; let model = info.model_name.as_deref().unwrap_or("(none)"); @@ -2677,11 +2846,21 @@ async fn run_acp_server() -> anyhow::Result<()> { }, agent_client_protocol::on_receive_request!(), ) + // Turn-affecting handlers below run in spawned tasks, serialized by + // `turn_lock`, so the dispatch loop stays free to route client + // responses (permission answers) while a turn is in flight. Awaiting a + // client request from *inside* a handler would deadlock: the dispatch + // loop can't read the response while the handler blocks it. .on_receive_request( { let state = Arc::clone(&state); async move |req: LoadSessionRequest, responder, cx: ConnectionTo<Client>| { - handle_response(responder, state.handle_load_session(&cx, req).await) + let state = Arc::clone(&state); + let task_cx = cx.clone(); + cx.spawn(async move { + let _turn = state.turn_lock.lock().await; + handle_response(responder, state.handle_load_session(&task_cx, req).await) + }) } }, agent_client_protocol::on_receive_request!(), @@ -2690,7 +2869,12 @@ async fn run_acp_server() -> anyhow::Result<()> { { let state = Arc::clone(&state); async move |req: ForkSessionRequest, responder, cx: ConnectionTo<Client>| { - handle_response(responder, state.handle_fork_session(&cx, req).await) + let state = Arc::clone(&state); + let task_cx = cx.clone(); + cx.spawn(async move { + let _turn = state.turn_lock.lock().await; + handle_response(responder, state.handle_fork_session(&task_cx, req).await) + }) } }, agent_client_protocol::on_receive_request!(), @@ -2699,7 +2883,12 @@ async fn run_acp_server() -> anyhow::Result<()> { { let state = Arc::clone(&state); async move |req: NewSessionRequest, responder, cx: ConnectionTo<Client>| { - handle_response(responder, state.handle_new_session(&cx, req).await) + let state = Arc::clone(&state); + let task_cx = cx.clone(); + cx.spawn(async move { + let _turn = state.turn_lock.lock().await; + handle_response(responder, state.handle_new_session(&task_cx, req).await) + }) } }, agent_client_protocol::on_receive_request!(), @@ -2708,7 +2897,12 @@ async fn run_acp_server() -> anyhow::Result<()> { { let state = Arc::clone(&state); async move |req: PromptRequest, responder, cx: ConnectionTo<Client>| { - handle_response(responder, state.handle_prompt(&cx, req).await) + let state = Arc::clone(&state); + let task_cx = cx.clone(); + cx.spawn(async move { + let _turn = state.turn_lock.lock().await; + handle_response(responder, state.handle_prompt(&task_cx, req).await) + }) } }, agent_client_protocol::on_receive_request!(), @@ -2719,10 +2913,15 @@ async fn run_acp_server() -> anyhow::Result<()> { async move |req: SetSessionConfigOptionRequest, responder, cx: ConnectionTo<Client>| { - handle_response( - responder, - state.handle_set_session_config_option(&cx, req).await, - ) + let state = Arc::clone(&state); + let task_cx = cx.clone(); + cx.spawn(async move { + let _turn = state.turn_lock.lock().await; + handle_response( + responder, + state.handle_set_session_config_option(&task_cx, req).await, + ) + }) } }, agent_client_protocol::on_receive_request!(),

new file mode 100644 index 0000000..c45c6b9 --- /dev/null +++ b/src/permissions.rs @@ -0,0 +1,285 @@ +//! Tool permission policy: which agent tools may run, and when to ask. +//! +//! Every tool call funnels through one decision point before execution +//! (`decision_for`). Tools are classified by risk: *read-only* tools (reading +//! files, searching, listing, fetching a web page) always run, while *mutating* +//! tools (writing files, deleting, shell commands, MCP tools) are governed by +//! policy. The policy layers, first match wins: +//! +//! 1. **Plan mode** — a per-session switch that denies every mutating tool with +//! a message telling the model to present a plan instead. Toggled via +//! `/plan on|off` (TUI and ACP). +//! 2. **Session grants** — "always allow this session", recorded when the user +//! picks that option in an approval prompt. +//! 3. **Per-tool override** — `[permissions.tools]` in `settings.toml`, e.g. +//! `run_command = "ask"`, `edit_file = "allow"`, `delete_file = "deny"`. +//! 4. **Default mode** — `[permissions] default = "ask"|"allow"|"deny"` in +//! `settings.toml`; `ask` on a fresh install. +//! +//! The `SIGIT_PERMISSIONS` env var (`allow`/`ask`/`deny`) overrides the stored +//! default without writing the file — the escape hatch for ACP clients that +//! cannot answer `session/request_permission` and for CI/headless runs. +//! +//! Tools discovered from MCP servers (`mcp__*`) and any unknown tool name are +//! treated as mutating: external tools can have arbitrary side effects, so the +//! safe assumption is to gate them. +//! +//! Session state (grants + plan mode) lives in a process-global keyed by +//! session id — the same pattern as `mcp.rs`'s server cache — so the ACP +//! multi-session surface and the single-session TUI share one implementation. + +use std::collections::{HashMap, HashSet}; +use std::sync::{Mutex, OnceLock}; + +use crate::settings::{self, PermissionMode}; + +/// Session key used by the interactive TUI, which only ever has one session. +pub const TUI_SESSION: &str = "tui"; + +/// How risky a tool is to run without the user's sign-off. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ToolRisk { + /// Observes state without changing it; always allowed to run. + ReadOnly, + /// Changes files, runs commands, or has unknown side effects; governed by + /// the permission policy. + Mutating, +} + +/// The outcome of the policy check for one tool call. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Decision { + /// Run the tool without asking. + Allow, + /// Ask the user before running (surface-specific: ACP permission request + /// or TUI approval prompt). + Ask, + /// Do not run the tool; the string is returned to the model as the tool + /// result so it can adapt instead of retrying blindly. + Deny(String), +} + +/// Classify a tool by name. Unknown names and MCP tools are mutating: the +/// conservative default for anything whose side effects we can't see. +pub fn classify(tool_name: &str) -> ToolRisk { + match tool_name { + "read_file" | "list_directory" | "search_files" | "glob" | "read_website" + | "write_todos" | "skill" => ToolRisk::ReadOnly, + _ => ToolRisk::Mutating, + } +} + +/// Per-session permission state. +#[derive(Default)] +struct SessionPerms { + /// Tools the user chose "always allow this session" for. + always_allow: HashSet<String>, + /// When set, every mutating tool is denied with a plan-mode message. + plan_mode: bool, +} + +fn sessions() -> &'static Mutex<HashMap<String, SessionPerms>> { + static SESSIONS: OnceLock<Mutex<HashMap<String, SessionPerms>>> = OnceLock::new(); + SESSIONS.get_or_init(|| Mutex::new(HashMap::new())) +} + +fn with_session<T>(session: &str, f: impl FnOnce(&mut SessionPerms) -> T) -> T { + let mut map = sessions() + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + f(map.entry(session.to_string()).or_default()) +} + +/// The message returned to the model when a mutating tool is blocked by plan +/// mode. Instructive rather than terse so the model changes course in one turn. +fn plan_mode_denial(tool_name: &str) -> String { + format!( + "Plan mode is active: `{tool_name}` was not executed because it modifies state. \ + Present a concise plan of the changes you intend to make and ask the user to \ + approve it (they can run /plan off to enable execution). Read-only tools \ + (read_file, search_files, glob, list_directory) remain available for research." + ) +} + +/// The message returned to the model when the user (or policy) denies a tool. +pub fn user_denial(tool_name: &str) -> String { + format!( + "The user denied permission to run `{tool_name}`. Do not retry the same call. \ + Explain what you wanted to do and ask the user how to proceed, or continue \ + with an approach that does not need this tool." + ) +} + +/// Policy check for one tool call. See the module docs for the layering. +pub fn decision_for(session: &str, tool_name: &str) -> Decision { + if classify(tool_name) == ToolRisk::ReadOnly { + return Decision::Allow; + } + + let (plan_mode, granted) = with_session(session, |s| { + (s.plan_mode, s.always_allow.contains(tool_name)) + }); + + if plan_mode { + return Decision::Deny(plan_mode_denial(tool_name)); + } + if granted { + return Decision::Allow; + } + + match settings::permission_mode_for(tool_name) { + PermissionMode::Allow => Decision::Allow, + PermissionMode::Ask => Decision::Ask, + PermissionMode::Deny => Decision::Deny(format!( + "`{tool_name}` is denied by the permission policy in settings.toml. \ + Do not retry it; work without this tool or ask the user to change \ + the policy." + )), + } +} + +/// Record an "always allow this session" grant for a tool. +pub fn grant_for_session(session: &str, tool_name: &str) { + with_session(session, |s| { + s.always_allow.insert(tool_name.to_string()); + }); +} + +/// Toggle plan mode for a session. Returns the new state. +pub fn set_plan_mode(session: &str, enabled: bool) -> bool { + with_session(session, |s| { + s.plan_mode = enabled; + s.plan_mode + }) +} + +/// Whether plan mode is active for a session. +pub fn plan_mode(session: &str) -> bool { + with_session(session, |s| s.plan_mode) +} + +/// Drop all recorded state for a session (fresh session, /clear, or session +/// teardown) so grants never outlive the conversation they were given in. +pub fn reset_session(session: &str) { + let mut map = sessions() + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + map.remove(session); +} + +/// One-line status summary for `/permissions` and `/status`. +pub fn describe(session: &str) -> String { + let plan = if plan_mode(session) { "on" } else { "off" }; + let default = settings::permission_default(); + let granted = with_session(session, |s| { + let mut names: Vec<&str> = s.always_allow.iter().map(String::as_str).collect(); + names.sort_unstable(); + names.join(", ") + }); + let granted = if granted.is_empty() { + "none".to_string() + } else { + granted + }; + format!( + "permissions: default={default} | plan mode: {plan} | session grants: {granted}\n\ + read-only tools always run; configure [permissions] in settings.toml" + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// `decision_for` reads settings (env + file), and the settings test + /// mutates `SIGIT_CONFIG_DIR`/`SIGIT_PERMISSIONS` under this lock — hold it + /// here too so parallel test runs don't race, and point the config dir at + /// an empty sandbox so a developer's real settings.toml can't skew results. + fn env_guard() -> std::sync::MutexGuard<'static, ()> { + let guard = crate::ENV_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let dir = std::env::temp_dir().join(format!("sigit_perm_tests_{}", std::process::id())); + // SAFETY: process-global env mutation, serialized by ENV_TEST_LOCK; the + // other env-touching tests re-set these before reading. + unsafe { std::env::set_var("SIGIT_CONFIG_DIR", &dir) }; + unsafe { std::env::remove_var("SIGIT_PERMISSIONS") }; + guard + } + + #[test] + fn read_only_tools_always_allowed() { + let _guard = env_guard(); + for tool in [ + "read_file", + "list_directory", + "search_files", + "glob", + "read_website", + "write_todos", + "skill", + ] { + assert_eq!(classify(tool), ToolRisk::ReadOnly, "{tool}"); + assert_eq!(decision_for("t-ro", tool), Decision::Allow, "{tool}"); + } + } + + #[test] + fn mutating_and_unknown_tools_are_gated() { + for tool in [ + "edit_file", + "multi_edit", + "create_file", + "create_directory", + "delete_file", + "run_command", + "remember", + "mcp__server__anything", + "totally_unknown_tool", + ] { + assert_eq!(classify(tool), ToolRisk::Mutating, "{tool}"); + } + } + + #[test] + fn plan_mode_denies_mutating_and_spares_read_only() { + let _guard = env_guard(); + let session = "t-plan"; + reset_session(session); + set_plan_mode(session, true); + assert!(matches!( + decision_for(session, "run_command"), + Decision::Deny(_) + )); + assert_eq!(decision_for(session, "read_file"), Decision::Allow); + set_plan_mode(session, false); + reset_session(session); + } + + #[test] + fn session_grant_short_circuits_ask() { + let _guard = env_guard(); + let session = "t-grant"; + reset_session(session); + grant_for_session(session, "edit_file"); + assert_eq!(decision_for(session, "edit_file"), Decision::Allow); + // Other tools are unaffected by the grant. + assert_ne!(decision_for(session, "delete_file"), Decision::Allow); + reset_session(session); + assert_ne!(decision_for(session, "edit_file"), Decision::Allow); + } + + #[test] + fn plan_mode_outranks_session_grant() { + let _guard = env_guard(); + let session = "t-rank"; + reset_session(session); + grant_for_session(session, "edit_file"); + set_plan_mode(session, true); + assert!(matches!( + decision_for(session, "edit_file"), + Decision::Deny(_) + )); + reset_session(session); + } +}

index 413cd60..716c4d8 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -5,12 +5,14 @@ //! [`crate::credentials`] but holds preferences rather than secrets, so it is //! not permission-restricted. //! -//! The only setting today is `local_inference`: whether on-device inference is -//! the active mode. It is the source of truth for the local/cloud toggle and -//! drives how `/models` presents the picker. It is stored locally so the toggle -//! works even on ACP clients that do not support slash commands (e.g. Xcode), -//! where it is also surfaced as a session config option. +//! Settings today: `local_inference` (whether on-device inference is the +//! active mode — the source of truth for the local/cloud toggle, also surfaced +//! as a session config option for ACP clients without slash commands, e.g. +//! Xcode) and `[permissions]` (the tool permission policy consumed by +//! `crate::permissions`: a default mode for mutating tools plus per-tool +//! overrides). +use std::collections::BTreeMap; use std::path::PathBuf; use serde::{Deserialize, Serialize}; @@ -20,10 +22,73 @@ use serde::{Deserialize, Serialize}; /// style); it never writes the file. const LOCAL_INFERENCE_ENV: &str = "SIGIT_LOCAL_INFERENCE"; +/// Env override for the default permission mode (`allow`/`ask`/`deny`). Wins +/// over the stored default (but not over per-tool overrides); never writes the +/// file. The escape hatch for ACP clients that cannot answer permission +/// requests and for headless runs. +const PERMISSIONS_ENV: &str = "SIGIT_PERMISSIONS"; + fn default_local_inference() -> bool { true } +/// What to do when the model calls a mutating tool. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] +#[serde(rename_all = "lowercase")] +pub enum PermissionMode { + /// Run without asking. + Allow, + /// Ask the user first (ACP permission request / TUI approval prompt). + #[default] + Ask, + /// Never run; the model gets an explanatory tool result. + Deny, +} + +impl PermissionMode { + fn parse(value: &str) -> Option<Self> { + match value.trim().to_ascii_lowercase().as_str() { + "allow" => Some(Self::Allow), + "ask" => Some(Self::Ask), + "deny" => Some(Self::Deny), + _ => None, + } + } +} + +impl std::fmt::Display for PermissionMode { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(match self { + Self::Allow => "allow", + Self::Ask => "ask", + Self::Deny => "deny", + }) + } +} + +/// The `[permissions]` table: a default mode for mutating tools plus per-tool +/// overrides, e.g. +/// +/// ```toml +/// [permissions] +/// default = "ask" +/// +/// [permissions.tools] +/// edit_file = "allow" +/// delete_file = "deny" +/// ``` +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] +pub struct PermissionSettings { + /// Mode for mutating tools without a per-tool override. `ask` on a fresh + /// install. + #[serde(default)] + pub default: PermissionMode, + /// Per-tool overrides by tool name (MCP tools use their full + /// `mcp__<server>__<tool>` name). + #[serde(default)] + pub tools: BTreeMap<String, PermissionMode>, +} + /// Persisted preferences. New fields must carry `#[serde(default)]` so older /// files keep deserializing. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -32,12 +97,16 @@ pub struct Settings { /// fresh install. #[serde(default = "default_local_inference")] pub local_inference: bool, + /// Permission policy for mutating agent tools. + #[serde(default)] + pub permissions: PermissionSettings, } impl Default for Settings { fn default() -> Self { Self { local_inference: default_local_inference(), + permissions: PermissionSettings::default(), } } } @@ -108,6 +177,32 @@ pub fn set_local_inference(enabled: bool) -> Result<(), String> { store(&settings) } +/// The default permission mode for mutating tools: the `SIGIT_PERMISSIONS` env +/// var when set to a recognized mode, else the stored `[permissions] default`. +pub fn permission_default() -> PermissionMode { + if let Ok(raw) = std::env::var(PERMISSIONS_ENV) + && let Some(mode) = PermissionMode::parse(&raw) + { + return mode; + } + load().permissions.default +} + +/// The effective permission mode for one tool: its `[permissions.tools]` +/// override when present, else the default (see [`permission_default`]). +pub fn permission_mode_for(tool_name: &str) -> PermissionMode { + let settings = load(); + if let Some(mode) = settings.permissions.tools.get(tool_name) { + return *mode; + } + if let Ok(raw) = std::env::var(PERMISSIONS_ENV) + && let Some(mode) = PermissionMode::parse(&raw) + { + return mode; + } + settings.permissions.default +} + #[cfg(test)] mod tests { use super::*; @@ -146,6 +241,40 @@ mod tests { "unrecognized env value falls back to stored setting" ); + // Permissions: fresh install asks; per-tool overrides win over the + // default; the env var overrides the stored default but not per-tool + // overrides. + unsafe { std::env::remove_var(PERMISSIONS_ENV) }; + assert_eq!(permission_default(), PermissionMode::Ask); + assert_eq!(permission_mode_for("run_command"), PermissionMode::Ask); + + let mut settings = load(); + settings.permissions.default = PermissionMode::Allow; + settings + .permissions + .tools + .insert("delete_file".to_string(), PermissionMode::Deny); + store(&settings).unwrap(); + assert_eq!(permission_default(), PermissionMode::Allow); + assert_eq!(permission_mode_for("run_command"), PermissionMode::Allow); + assert_eq!(permission_mode_for("delete_file"), PermissionMode::Deny); + + unsafe { std::env::set_var(PERMISSIONS_ENV, "deny") }; + assert_eq!(permission_default(), PermissionMode::Deny); + assert_eq!(permission_mode_for("run_command"), PermissionMode::Deny); + assert_eq!( + permission_mode_for("delete_file"), + PermissionMode::Deny, + "per-tool override still wins" + ); + unsafe { std::env::set_var(PERMISSIONS_ENV, "garbage") }; + assert_eq!( + permission_default(), + PermissionMode::Allow, + "unrecognized env value falls back to stored setting" + ); + + unsafe { std::env::remove_var(PERMISSIONS_ENV) }; unsafe { std::env::remove_var(LOCAL_INFERENCE_ENV) }; unsafe { std::env::remove_var("SIGIT_CONFIG_DIR") }; let _ = std::fs::remove_dir_all(&dir);