diff --git a/CONFIG.md b/CONFIG.md index b2aa89d5..a7b35ac2 100644 --- a/CONFIG.md +++ b/CONFIG.md @@ -90,12 +90,27 @@ Accepted top-level keys: Permission actions are lowercase strings: `allow`, `ask`, or `deny`. Each tool rule can be a single action or an object mapping glob-like patterns to actions. Supported permission tool keys are `bash`, `read`, `write`, `edit`, `grep`, -`find_files`, `list_dir`, and `write_todo_list`. MCP-backed tools are -checked under `mcp_tool:{server_name}:{tool_name}`. Use `"*"` for the -default action, `external_directory` for absolute-path rules outside the -working directory, and `doom_loop` for repeated identical tool calls -(default: `ask`). If `bash` is omitted, dirge installs its built-in -safe bash allow/deny rules. +`find_files`, `list_dir`, `write_todo_list`, `apply_patch`, `lsp`, and +`question`. MCP-backed tools are checked under +`mcp_tool:{server_name}:{tool_name}`. Use `"*"` for the default action, +`external_directory` for absolute-path rules outside the working directory, +and `doom_loop` for repeated identical tool calls (default: `ask`). If +`bash` is omitted, dirge installs its built-in safe bash allow/deny rules. + +### Mode semantics + +- **`standard`** (default): every rule in `permission` is consulted; tools without + matching rules fall back to `*` (default `allow`). +- **`restrictive`**: like `standard`, but any tool whose rule resolves to `allow` + via the `*` fallback (no explicit allow rule matched) is converted to `ask`. + Explicit `allow` rules still allow. Explicit `deny` rules still deny. +- **`accept`** (equivalent to `--accept-all`): auto-allows tools whose targets + resolve inside the working directory; tools touching paths outside still + consult `external_directory` rules. +- **`yolo`** (equivalent to `--yolo`): bypasses every check. Use with caution. + +CLI precedence (high → low): `--yolo` > `--accept-all` > `--restrictive` > +`default_permission_mode` config > `standard`. When compiled with MCP support, `mcp_servers` accepts command-based and URL-based servers: diff --git a/src/permission/checker.rs b/src/permission/checker.rs index 850387f5..fe6a48c6 100644 --- a/src/permission/checker.rs +++ b/src/permission/checker.rs @@ -29,7 +29,10 @@ pub struct PermissionChecker { /// classic glob semantics (one segment, doesn't cross `/`). Everything else /// is treated as shell/text where `*` means "any chars including /". pub(crate) fn is_path_tool_name(tool: &str) -> bool { - matches!(tool, "read" | "write" | "edit" | "list_dir") + matches!( + tool, + "read" | "write" | "edit" | "list_dir" | "apply_patch" | "lsp" + ) } /// Build a Pattern with the right `*` semantics for the given tool. @@ -291,7 +294,12 @@ impl PermissionChecker { } fn is_path_tool(&self, tool: &str) -> bool { - matches!(tool, "read" | "write" | "edit" | "list_dir") + // Must match `is_path_tool_name` — these are the tools that + // take a filesystem path as their permission input and need + // `external_directory` rule consultation. `apply_patch` and + // `lsp` are included because both route filesystem-path + // strings through `check_perm_path`. + is_path_tool_name(tool) } fn is_external_path(&self, path_str: &str) -> bool { diff --git a/src/permission/mod.rs b/src/permission/mod.rs index 7e8e6c5d..077073b1 100644 --- a/src/permission/mod.rs +++ b/src/permission/mod.rs @@ -46,11 +46,28 @@ pub struct PermissionConfig { pub doom_loop: Option, } +/// Per-session security mode. Selected via `--yolo` / `--accept-all` / +/// `--restrictive` CLI flags or the `default_permission_mode` config +/// key. Mode precedence (high to low): `Yolo > Accept > Restrictive > +/// Standard`. #[derive(Debug, Clone, Copy, PartialEq)] pub enum SecurityMode { + /// Every rule in `PermissionConfig` is consulted; tools with no + /// matching rule fall back to the `*` default action. Standard, + /// Like `Standard`, but any tool whose rule resolves to `Allow` + /// *via the `*` fallback* (no explicit allow rule matched) gets + /// upgraded to `Ask`. Explicit allow rules still allow; explicit + /// deny rules still deny. The semantic difference from + /// `Standard`: "if nothing explicitly approved this, ask the + /// user." It does NOT flip every Allow to Ask. Restrictive, + /// Auto-allows tools whose targets resolve inside the working + /// directory; tools touching paths outside `cwd` still consult + /// `external_directory` rules. Useful for fast iteration on a + /// trusted project. Accept, + /// Bypasses every check. Use with caution. Yolo, } diff --git a/src/session/storage.rs b/src/session/storage.rs index c7add69a..a0a540a2 100644 --- a/src/session/storage.rs +++ b/src/session/storage.rs @@ -62,7 +62,21 @@ pub fn save_session(session: &Session) -> anyhow::Result<()> { // over the target. A crash mid-write leaves the temp behind but // never a truncated `.json`. The rename is atomic on every OS we // target. Use the same parent dir so rename stays on one filesystem. - let tmp = dir.join(format!(".{}.json.tmp", session.id)); + // + // The tmp filename includes a per-call nonce (pid + nanos) so two + // concurrent saves of the same session id don't collide on the + // tmp file. Each thread/process writes to its own tmp; the rename + // race is harmless (last-writer wins on the target, but neither + // tmp is partial because each was fully written before rename). + let nonce = format!( + "{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_nanos()) + .unwrap_or(0), + ); + let tmp = dir.join(format!(".{}.{}.json.tmp", session.id, nonce)); { use std::io::Write; let mut f = std::fs::File::create(&tmp)?; diff --git a/src/ui/markdown.rs b/src/ui/markdown.rs index e8ee5b89..5640f4a4 100644 --- a/src/ui/markdown.rs +++ b/src/ui/markdown.rs @@ -94,6 +94,13 @@ fn render_table( widths[i] = widths[i].max(cell.chars().count()); } } + // Minimum column width — ragged rows (one row has fewer cells + // than `ncols`) would otherwise leave `widths[i] = 0` for the + // missing columns, breaking the separator line + right-border + // alignment. Guarantee at least 1 char per column. + for w in widths.iter_mut() { + *w = (*w).max(1); + } // Cap any single column to avoid one runaway cell blowing the // line width. Distribute available width: target inner width = // max_width - 4 (for outer `| ` + ` |`), minus 3*(ncols-1) for @@ -404,8 +411,13 @@ pub fn markdown_to_styled(text: &str, max_width: usize) -> Vec { } } Event::SoftBreak | Event::HardBreak => { - if in_code_block { - acc.push('\n'); + if in_table { + // Break inside a table cell would smear the cell + // across multiple lines and misalign the row. + // Markdown spec doesn't allow real newlines in + // table cells — substitute a space so the visible + // content stays on one line. + current_cell.push(' '); } else { acc.push('\n'); } diff --git a/src/ui/slash.rs b/src/ui/slash.rs index bf8d8d63..cecc429f 100644 --- a/src/ui/slash.rs +++ b/src/ui/slash.rs @@ -1050,11 +1050,21 @@ pub async fn handle_slash( .cloned(); match last_user { Some(msg) => { - // Pop the trailing assistant response (if any) so a - // /retry doesn't leave the failed reply in the - // session — the agent would otherwise see its own - // bad answer as context on the retry. - let _ = undo_last(session); + // Pop messages until we've removed the last user + // message itself — covers assistant replies AND + // system messages (compress summaries, error + // notices) that landed between the user prompt + // and the current state. `undo_last` only handles + // the Assistant/User pair pattern; using it here + // would leave a trailing System message in the + // session and the agent would see it on retry. + while let Some(last) = session.messages.last() { + let was_user = last.role == MessageRole::User; + session.pop_last_message(); + if was_user { + break; + } + } input.buffer = msg.content.clone(); input.cursor = msg.content.len(); render_session(renderer, session, cli, cfg, context)?;