From 6ce60eafde9f0790bdb471c9004b2ba210fd4a3d Mon Sep 17 00:00:00 2001 From: greyfreedom Date: Fri, 24 Jul 2026 18:21:20 +0800 Subject: [PATCH 1/3] feat(tui): persist exact repo-scoped allow grants Add a distinct approval-card action for remembering eligible safe shell and file-write calls as typed allow rules. Scope remembered grants to the active workspace, require exact shell command matching, preserve platform-correct path boundaries, and keep dangerous, critical, hook, auto-review, and repo-law holds outside the bypass. --- CHANGELOG.md | 7 + config.example.toml | 12 +- crates/config/src/lib.rs | 113 ++++++++- crates/config/src/tests.rs | 114 +++++++++ crates/execpolicy/src/lib.rs | 255 ++++++++++++++++++- crates/tui/CHANGELOG.md | 13 +- crates/tui/locales/en.json | 6 +- crates/tui/locales/es-419.json | 6 +- crates/tui/locales/ja.json | 6 +- crates/tui/locales/ko.json | 6 +- crates/tui/locales/pt-BR.json | 6 +- crates/tui/locales/vi.json | 6 +- crates/tui/locales/zh-Hans.json | 6 +- crates/tui/locales/zh-Hant.json | 6 +- crates/tui/src/core/engine.rs | 39 ++- crates/tui/src/core/engine/tests.rs | 88 +++++++ crates/tui/src/core/engine/turn_loop.rs | 10 + crates/tui/src/localization.rs | 4 + crates/tui/src/tui/approval.rs | 316 +++++++++++++++++++++--- crates/tui/src/tui/ui.rs | 38 ++- crates/tui/src/tui/ui/tests.rs | 75 +++++- crates/tui/src/tui/views/mod.rs | 4 +- crates/tui/src/tui/widgets/mod.rs | 101 +++++--- docs/CONFIGURATION.md | 30 ++- 24 files changed, 1128 insertions(+), 139 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a01224a0e8..e85d82c7ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Approval cards can now remember eligible safe shell and file-write approvals + as exact `allow` rules scoped to the current repository. Remembered shell + commands use complete-command matching, validated file and patch paths remain + workspace-relative, and dangerous, critical, or repo-law-held requests stay + ineligible and continue to require review. ## [0.9.1] - 2026-07-24 diff --git a/config.example.toml b/config.example.toml index a2b647f807..a724fb6c20 100644 --- a/config.example.toml +++ b/config.example.toml @@ -184,12 +184,16 @@ sandbox_mode = "workspace-write" # read-only | workspace-write | danger-full-acc # Typed permission rules live in a sibling `permissions.toml` file, not in # config.toml. Each `[[rules]]` entry accepts `tool`, optional `command` -# or `path`, and an `action` field: `"deny"`, `"ask"` (default), or `"allow"`. +# or `path`, optional absolute `workspace`, optional `command_exact = true`, +# and an `action` field: `"deny"`, `"ask"` (default), or `"allow"`. # Deny always wins over ask, which wins over allow. Globs, broad directory # rules, and a rule editor/deleter UI are future work. # -# In supported approval cards, press `S` to approve once and save exact rules -# (currently saved as ask-only; deny/allow UI save is future work): +# In supported approval cards, press `S` to allow once and save an exact ask +# rule. Eligible safe calls also offer `P` / "Always allow this exact rule in +# this repo", which saves an exact `allow` rule scoped to the current absolute +# workspace. Dangerous/critical calls and repo-law prompts cannot save allow +# grants. The UI still does not save deny rules or provide editing/deletion: # exec_shell -> exact approved command string # write_file -> exact workspace-relative target path # edit_file -> exact workspace-relative target path @@ -219,6 +223,8 @@ sandbox_mode = "workspace-write" # read-only | workspace-write | danger-full-acc # [[rules]] # tool = "exec_shell" # command = "git status" +# command_exact = true +# workspace = "/absolute/path/to/project" # action = "allow" # # # Path-based deny diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index 99c3f07b6c..6ce16a3c43 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -53,7 +53,7 @@ use std::sync::OnceLock; use anyhow::{Context, Result, bail}; pub use auth_source::{AuthSourceKind, ProviderAuthSourceToml}; pub use codewhale_execpolicy::ToolAskRule; -use codewhale_execpolicy::{ExecPolicyEngine, Ruleset}; +use codewhale_execpolicy::{ExecPolicyEngine, PermissionAction, Ruleset}; use codewhale_secrets::SecretSource; pub use codewhale_secrets::Secrets; pub use external_credentials::{ @@ -360,8 +360,8 @@ pub struct ProvidersToml { /// Sibling `permissions.toml` schema. /// /// Each rule is a typed condition that can deny, allow, or ask before a tool -/// invocation. UI actions that persist deny/allow rules are future work; the -/// approval card still saves ask rules. +/// invocation. The approval card persists ask rules and narrowly scoped, +/// exact allow grants; deny rules remain manually authored. #[derive(Debug, Clone, Serialize, Deserialize, Default, PartialEq, Eq)] #[serde(deny_unknown_fields)] pub struct PermissionsToml { @@ -377,7 +377,6 @@ impl PermissionsToml { #[must_use] pub fn ruleset(&self) -> Ruleset { - use codewhale_execpolicy::PermissionAction; let mut denied = Vec::new(); let mut trusted = Vec::new(); let mut ask_rules = Vec::new(); @@ -387,7 +386,10 @@ impl PermissionsToml { PermissionAction::Deny => { // Command-based deny rules are promoted to denied_prefixes // so they are caught by execpolicy's deny-always-wins check. - if let Some(cmd) = &rule.command { + if let Some(cmd) = &rule.command + && !rule.command_exact + && rule.workspace.is_none() + { denied.push(cmd.clone()); } // Always keep in ask_rules for path-based and tool-only matching. @@ -397,7 +399,10 @@ impl PermissionsToml { // Command-based allow rules are promoted to trusted_prefixes // for arity-aware matching. Path-only allow rules are // handled through ask_rules (they skip the approval prompt). - if let Some(cmd) = &rule.command { + if let Some(cmd) = &rule.command + && !rule.command_exact + && rule.workspace.is_none() + { trusted.push(cmd.clone()); } // Keep in ask_rules so path-only allow rules also work. @@ -3684,9 +3689,62 @@ impl ConfigStore { /// are ignored, and the in-memory permissions snapshot is refreshed after /// a successful write. pub fn append_ask_rules(&mut self, rules: &[ToolAskRule]) -> Result { + self.append_permission_rules(rules, PermissionAction::Ask) + } + + /// Atomically append exact, repo-scoped allow rules to the sibling + /// `permissions.toml` file. + /// + /// The caller is responsible for deciding which tool calls are eligible; + /// this boundary rejects broad or incorrectly typed records so a UI bug + /// cannot persist an unscoped allow grant. + pub fn append_allow_rules(&mut self, rules: &[ToolAskRule]) -> Result { + for rule in rules { + if rule.action != PermissionAction::Allow { + bail!("append_allow_rules only accepts action = \"allow\""); + } + let Some(workspace) = rule + .workspace + .as_deref() + .and_then(codewhale_execpolicy::normalize_workspace_scope) + else { + bail!("persistent allow rules must be scoped to a workspace"); + }; + if rule.command.is_some() && !rule.command_exact { + bail!("persistent command allow rules must use exact matching"); + } + if rule.command.is_none() && rule.path.is_none() { + bail!("persistent allow rules must match an exact command or path"); + } + if let Some(command) = rule.command.as_deref() + && command.trim().is_empty() + { + bail!("persistent command allow rules must not be empty"); + } + if let Some(path) = rule.path.as_deref() + && codewhale_execpolicy::normalize_workspace_relative_path(path, &workspace) + .is_none_or(|path| path.is_empty()) + { + bail!("persistent path allow rules must stay within the workspace"); + } + } + self.append_permission_rules(rules, PermissionAction::Allow) + } + + fn append_permission_rules( + &mut self, + rules: &[ToolAskRule], + expected_action: PermissionAction, + ) -> Result { if rules.is_empty() { return Ok(0); } + if rules.iter().any(|rule| rule.action != expected_action) { + bail!( + "permission rule action does not match requested {:?} persistence", + expected_action + ); + } let path = checked_permissions_path_for_config_path(&self.path)?; let raw = if checked_path_exists(&path)? { @@ -3727,7 +3785,7 @@ impl ConfigStore { if permissions.rules.contains(rule) { continue; } - append_ask_rule(rules_item, rule)?; + append_permission_rule(rules_item, rule)?; permissions.rules.push(rule.clone()); added += 1; } @@ -4350,44 +4408,75 @@ fn load_sibling_permissions(config_path: &Path) -> Result { }) } -fn append_ask_rule(item: &mut toml_edit::Item, rule: &ToolAskRule) -> Result<()> { +fn append_permission_rule(item: &mut toml_edit::Item, rule: &ToolAskRule) -> Result<()> { match item { toml_edit::Item::ArrayOfTables(rules) => { - rules.push(ask_rule_table(rule)); + rules.push(permission_rule_table(rule)); Ok(()) } toml_edit::Item::Value(value) => { let Some(rules) = value.as_array_mut() else { bail!("`rules` in permissions.toml must be an array"); }; - rules.push(toml_edit::Value::InlineTable(ask_rule_inline_table(rule))); + rules.push(toml_edit::Value::InlineTable(permission_rule_inline_table( + rule, + ))); Ok(()) } _ => bail!("`rules` in permissions.toml must be an array"), } } -fn ask_rule_table(rule: &ToolAskRule) -> toml_edit::Table { +fn permission_rule_table(rule: &ToolAskRule) -> toml_edit::Table { let mut table = toml_edit::Table::new(); table["tool"] = toml_edit::value(rule.tool.clone()); if let Some(command) = rule.command.as_deref() { table["command"] = toml_edit::value(command); } + if rule.command_exact { + table["command_exact"] = toml_edit::value(true); + } if let Some(path) = rule.path.as_deref() { table["path"] = toml_edit::value(path); } + if let Some(workspace) = rule.workspace.as_deref() { + table["workspace"] = toml_edit::value(workspace); + } + if rule.action != PermissionAction::Ask { + table["action"] = toml_edit::value(match rule.action { + PermissionAction::Allow => "allow", + PermissionAction::Ask => "ask", + PermissionAction::Deny => "deny", + }); + } table } -fn ask_rule_inline_table(rule: &ToolAskRule) -> toml_edit::InlineTable { +fn permission_rule_inline_table(rule: &ToolAskRule) -> toml_edit::InlineTable { let mut table = toml_edit::InlineTable::new(); table.insert("tool", toml_edit::Value::from(rule.tool.clone())); if let Some(command) = rule.command.as_deref() { table.insert("command", toml_edit::Value::from(command)); } + if rule.command_exact { + table.insert("command_exact", toml_edit::Value::from(true)); + } if let Some(path) = rule.path.as_deref() { table.insert("path", toml_edit::Value::from(path)); } + if let Some(workspace) = rule.workspace.as_deref() { + table.insert("workspace", toml_edit::Value::from(workspace)); + } + if rule.action != PermissionAction::Ask { + table.insert( + "action", + toml_edit::Value::from(match rule.action { + PermissionAction::Allow => "allow", + PermissionAction::Ask => "ask", + PermissionAction::Deny => "deny", + }), + ); + } table } diff --git a/crates/config/src/tests.rs b/crates/config/src/tests.rs index f5081f1896..f29c159ffd 100644 --- a/crates/config/src/tests.rs +++ b/crates/config/src/tests.rs @@ -176,6 +176,23 @@ fn permissions_ruleset_populates_denied_and_trusted_prefixes() { assert!(!ruleset.denied_prefixes.contains(&"cargo test".to_string())); } +#[test] +fn exact_workspace_command_allow_is_not_promoted_to_global_prefix() { + let rule = + ToolAskRule::exec_shell("cargo test").into_exact_workspace_allow("/workspace/project"); + let permissions = PermissionsToml { + rules: vec![rule.clone()], + }; + + let ruleset = permissions.ruleset(); + + assert!( + ruleset.trusted_prefixes.is_empty(), + "an exact repo grant must not become a global trusted prefix" + ); + assert_eq!(ruleset.ask_rules, vec![rule]); +} + #[test] fn permissions_ruleset_deny_without_command_stays_in_ask_rules() { // Tool-only deny (no command) can't be promoted to denied_prefixes. @@ -695,6 +712,103 @@ fn config_store_appends_ask_rule_to_inline_rules_array() { ); } +#[test] +fn config_store_appends_exact_workspace_allow_rules() { + let dir = tempfile::tempdir().expect("tempdir"); + let config_path = dir.path().join(CONFIG_FILE_NAME); + let permissions_path = dir.path().join(PERMISSIONS_FILE_NAME); + fs::write(&permissions_path, "# remembered grants stay user-owned\n") + .expect("write permissions"); + let mut store = ConfigStore::load(Some(config_path.clone())).expect("load config store"); + let rule = ToolAskRule::exec_shell("cargo test") + .into_exact_workspace_allow(dir.path().to_string_lossy()); + + assert_eq!( + store + .append_allow_rules(&[rule.clone(), rule.clone()]) + .expect("append allow rule"), + 1 + ); + + let body = fs::read_to_string(&permissions_path).expect("read permissions"); + assert!(body.contains("# remembered grants stay user-owned")); + assert!(body.contains("action = \"allow\"")); + assert!(body.contains("command_exact = true")); + assert!(body.contains(&format!("workspace = {:?}", dir.path().to_string_lossy()))); + let parsed: PermissionsToml = toml::from_str(&body).expect("parse permissions"); + assert_eq!(parsed.rules, vec![rule.clone()]); + + let exact = store + .exec_policy_engine() + .check(codewhale_execpolicy::ExecPolicyContext { + command: "cargo test", + cwd: dir.path().to_string_lossy().as_ref(), + tool: Some("exec_shell"), + path: None, + ask_for_approval: codewhale_execpolicy::AskForApproval::OnRequest, + sandbox_mode: Some("workspace-write"), + }) + .expect("check exact grant"); + assert_eq!( + exact.matched_action, + Some(codewhale_execpolicy::PermissionAction::Allow) + ); + assert!(!exact.requires_approval); + + let extra_args = store + .exec_policy_engine() + .check(codewhale_execpolicy::ExecPolicyContext { + command: "cargo test --workspace", + cwd: dir.path().to_string_lossy().as_ref(), + tool: Some("exec_shell"), + path: None, + ask_for_approval: codewhale_execpolicy::AskForApproval::OnRequest, + sandbox_mode: Some("workspace-write"), + }) + .expect("check extra args"); + assert!(extra_args.requires_approval); + + let reloaded = ConfigStore::load(Some(config_path)).expect("reload config store"); + assert_eq!(reloaded.permissions().rules, vec![rule]); +} + +#[test] +fn config_store_rejects_broad_or_unscoped_allow_rules() { + let dir = tempfile::tempdir().expect("tempdir"); + let config_path = dir.path().join(CONFIG_FILE_NAME); + let mut store = ConfigStore::load(Some(config_path)).expect("load config store"); + + let mut unscoped = ToolAskRule::exec_shell("cargo test"); + unscoped.action = codewhale_execpolicy::PermissionAction::Allow; + assert!( + store + .append_allow_rules(&[unscoped]) + .expect_err("unscoped allow must fail") + .to_string() + .contains("scoped to a workspace") + ); + + let mut prefix = ToolAskRule::exec_shell("cargo test"); + prefix.action = codewhale_execpolicy::PermissionAction::Allow; + prefix.workspace = Some(dir.path().to_string_lossy().into_owned()); + assert!( + store + .append_allow_rules(&[prefix]) + .expect_err("prefix allow must fail") + .to_string() + .contains("exact matching") + ); + + assert!( + store + .append_allow_rules(&[ToolAskRule::new("exec_shell") + .into_exact_workspace_allow(dir.path().to_string_lossy())]) + .expect_err("tool-wide allow must fail") + .to_string() + .contains("exact command or path") + ); +} + #[test] fn config_store_does_not_overwrite_invalid_permissions_file() { let dir = tempfile::tempdir().expect("tempdir"); diff --git a/crates/execpolicy/src/lib.rs b/crates/execpolicy/src/lib.rs index 68f5e5a1d0..426be485eb 100644 --- a/crates/execpolicy/src/lib.rs +++ b/crates/execpolicy/src/lib.rs @@ -104,9 +104,20 @@ pub struct ToolAskRule { /// Optional command prefix to match against (uses arity-aware matching). #[serde(default, skip_serializing_if = "Option::is_none")] pub command: Option, + /// Match `command` as the complete invocation instead of as a prefix. + /// + /// Approval-card remembered grants set this so approving one safe command + /// cannot silently authorize a later invocation with extra arguments. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub command_exact: bool, /// Optional file path pattern to match against. #[serde(default, skip_serializing_if = "Option::is_none")] pub path: Option, + /// Optional absolute workspace root that limits this rule to one repo. + /// + /// Rules authored without a workspace retain the historical global scope. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub workspace: Option, /// Action when this rule matches. Default: `"ask"` (backward compatible). #[serde(default = "default_rule_action")] pub action: PermissionAction, @@ -118,7 +129,9 @@ impl ToolAskRule { Self { tool: tool.into(), command: None, + command_exact: false, path: None, + workspace: None, action: PermissionAction::Ask, } } @@ -128,7 +141,9 @@ impl ToolAskRule { Self { tool: "exec_shell".to_string(), command: Some(command.into()), + command_exact: false, path: None, + workspace: None, action: PermissionAction::Ask, } } @@ -138,19 +153,36 @@ impl ToolAskRule { Self { tool: tool.into(), command: None, + command_exact: false, path: Some(path.into()), + workspace: None, action: PermissionAction::Ask, } } + /// Convert an exact rule candidate into a repo-scoped persistent allow. + #[must_use] + pub fn into_exact_workspace_allow(mut self, workspace: impl Into) -> Self { + self.command_exact = self.command.is_some(); + self.workspace = Some(workspace.into()); + self.action = PermissionAction::Allow; + self + } + fn label(&self) -> String { let mut parts = vec![format!("tool={}", self.tool)]; if let Some(command) = &self.command { parts.push(format!("command={command}")); } + if self.command_exact { + parts.push("command_exact=true".to_string()); + } if let Some(path) = &self.path { parts.push(format!("path={path}")); } + if let Some(workspace) = &self.workspace { + parts.push(format!("workspace={workspace}")); + } parts.join(" ") } } @@ -354,7 +386,13 @@ impl ExecPolicyEngine { .map(move |rule| (ruleset.layer, rule)) }) .filter(|(_, rule)| rule.tool == tool) + .filter(|(_, rule)| { + rule.workspace + .as_deref() + .is_none_or(|workspace| workspace_scope_matches(workspace, ctx.cwd)) + }) .filter(|(_, rule)| match rule.command.as_deref() { + Some(command) if rule.command_exact => command.trim() == ctx.command.trim(), Some(command) => self.arity_dict.allow_rule_matches(command, ctx.command), None => true, }) @@ -649,8 +687,10 @@ fn first_token(command: &str) -> String { /// the path is empty, traversing, drive-relative, or outside the workspace and /// must not be turned into a rule. pub fn normalize_workspace_relative_path(value: &str, workspace_root: &str) -> Option { - let path = parse_path_for_matching(value)?; - let workspace = parse_path_for_matching(workspace_root)?; + let workspace_uses_windows_paths = + is_windows_absolute_path(&workspace_root.trim().replace('\\', "/")); + let path = parse_path_for_matching(value, workspace_uses_windows_paths)?; + let workspace = parse_path_for_matching(workspace_root, workspace_uses_windows_paths)?; let workspace_root = workspace.root.as_ref()?; let relative_components = match path.root.as_ref() { @@ -666,14 +706,70 @@ pub fn normalize_workspace_relative_path(value: &str, workspace_root: &str) -> O Some(relative_components.join("/")) } +/// Return a stable absolute workspace scope suitable for a persisted rule. +/// +/// Relative paths and filesystem roots are rejected: remembered grants must +/// name one concrete repository rather than accidentally applying everywhere. +pub fn normalize_workspace_scope(value: &str) -> Option { + let value = value.trim().replace('\\', "/"); + if value.is_empty() { + return None; + } + + let (root, components) = if let Some(path) = value.strip_prefix('/') { + ("/".to_string(), path.to_string()) + } else if is_windows_absolute_path(&value) { + // Windows paths are case-insensitive in the environments CodeWhale + // supports. Keep the POSIX branch case-sensitive so two distinct + // repositories on a case-sensitive filesystem cannot share a grant. + let value = value.to_ascii_lowercase(); + (value[..2].to_string(), value[3..].to_string()) + } else { + return None; + }; + + let mut normalized_components = Vec::new(); + for component in components.split('/') { + match component { + "" | "." => {} + ".." => return None, + component => normalized_components.push(component), + } + } + if normalized_components.is_empty() { + return None; + } + + let separator = if root == "/" { "" } else { "/" }; + Some(format!( + "{root}{separator}{}", + normalized_components.join("/") + )) +} + +fn workspace_scope_matches(rule_workspace: &str, cwd: &str) -> bool { + match ( + normalize_workspace_scope(rule_workspace), + normalize_workspace_scope(cwd), + ) { + (Some(rule_workspace), Some(cwd)) => rule_workspace == cwd, + _ => false, + } +} + #[derive(Debug)] struct PathForMatching { root: Option, components: Vec, } -fn parse_path_for_matching(value: &str) -> Option { - let value = value.trim().replace('\\', "/").to_ascii_lowercase(); +fn parse_path_for_matching(value: &str, case_insensitive: bool) -> Option { + let value = value.trim().replace('\\', "/"); + let value = if case_insensitive { + value.to_ascii_lowercase() + } else { + value + }; if value.is_empty() { return None; } @@ -722,6 +818,11 @@ fn ask_rule_specificity(rule: &ToolAskRule) -> usize { .as_ref() .map_or(0, |command| command.len() + 1000) + rule.path.as_ref().map_or(0, |path| path.len() + 1000) + + rule + .workspace + .as_ref() + .map_or(0, |workspace| workspace.len() + 1000) + + usize::from(rule.command_exact) } #[cfg(test)] @@ -1229,6 +1330,7 @@ mod tests { command: Some("sed".into()), path: None, action: PermissionAction::Deny, + ..ToolAskRule::new("") }], )]); @@ -1264,6 +1366,7 @@ mod tests { command: Some("git status".into()), path: None, action: PermissionAction::Allow, + ..ToolAskRule::new("") }], )]); @@ -1316,12 +1419,14 @@ mod tests { command: Some("git status".into()), path: None, action: PermissionAction::Ask, + ..ToolAskRule::new("") }]), Ruleset::user(vec![], vec![]).with_ask_rules(vec![ToolAskRule { tool: "exec_shell".into(), command: Some("git status".into()), path: None, action: PermissionAction::Allow, + ..ToolAskRule::new("") }]), ]); @@ -1391,6 +1496,7 @@ mod tests { command: Some("sed".into()), path: None, action: PermissionAction::Deny, + ..ToolAskRule::new("") }); // exact match @@ -1411,6 +1517,7 @@ mod tests { command: Some("sed".into()), path: None, action: PermissionAction::Deny, + ..ToolAskRule::new("") }); // unrelated command passes through @@ -1428,6 +1535,7 @@ mod tests { command: Some("rm".into()), path: None, action: PermissionAction::Deny, + ..ToolAskRule::new("") }); assert!(!engine.check(ctx("rm -rf /", UnlessTrusted)).unwrap().allow); @@ -1448,6 +1556,7 @@ mod tests { command: Some("git push".into()), path: None, action: PermissionAction::Deny, + ..ToolAskRule::new("") }); assert!(!engine.check(ctx("git push", UnlessTrusted)).unwrap().allow); @@ -1473,6 +1582,7 @@ mod tests { command: Some("git push".into()), path: None, action: PermissionAction::Deny, + ..ToolAskRule::new("") }); assert!(engine.check(ctx("git pull", UnlessTrusted)).unwrap().allow); @@ -1517,12 +1627,14 @@ mod tests { command: Some("sed".into()), path: None, action: PermissionAction::Allow, + ..ToolAskRule::new("") }, ToolAskRule { tool: "exec_shell".into(), command: Some("sed".into()), path: None, action: PermissionAction::Deny, + ..ToolAskRule::new("") }, ], )]); @@ -1545,12 +1657,14 @@ mod tests { command: Some("sed".into()), path: None, action: PermissionAction::Deny, + ..ToolAskRule::new("") }, ToolAskRule { tool: "exec_shell".into(), command: Some("sed".into()), path: None, action: PermissionAction::Allow, + ..ToolAskRule::new("") }, ], )]); @@ -1572,12 +1686,14 @@ mod tests { command: None, path: Some("src/secrets.rs".into()), action: PermissionAction::Deny, + ..ToolAskRule::new("") }, ToolAskRule { tool: "write_file".into(), command: None, path: Some("src/secrets.rs".into()), action: PermissionAction::Allow, + ..ToolAskRule::new("") }, ], )]); @@ -1785,6 +1901,7 @@ mod tests { command: None, path: None, action: PermissionAction::Deny, + ..ToolAskRule::new("") }); // any exec_shell command should be blocked @@ -1817,6 +1934,7 @@ mod tests { command: Some("cargo".into()), path: None, action: PermissionAction::Allow, + ..ToolAskRule::new("") }); let d = engine @@ -1834,6 +1952,7 @@ mod tests { command: Some("git status".into()), path: None, action: PermissionAction::Allow, + ..ToolAskRule::new("") }); let d = engine.check(ctx("git status --short", OnRequest)).unwrap(); @@ -1848,6 +1967,7 @@ mod tests { command: Some("git status".into()), path: None, action: PermissionAction::Allow, + ..ToolAskRule::new("") }); // Unrelated command: normal approval flow applies. @@ -1866,6 +1986,7 @@ mod tests { command: Some("cargo".into()), path: None, action: PermissionAction::Allow, + ..ToolAskRule::new("") }); let d = engine.check(ctx("cargo check", Never)).unwrap(); @@ -1882,6 +2003,7 @@ mod tests { command: Some("cargo test".into()), path: None, action: PermissionAction::Ask, + ..ToolAskRule::new("") }); // Under UnlessTrusted: ask rule forces approval @@ -1913,6 +2035,7 @@ mod tests { command: Some("sed".into()), path: None, action: PermissionAction::Deny, + ..ToolAskRule::new("") }); let d = engine @@ -1960,6 +2083,128 @@ mod tests { assert!(d.requires_approval, "untrusted cmd needs approval"); } + #[test] + fn exact_workspace_allow_matches_only_the_same_command_and_repo() { + let rule = ToolAskRule::exec_shell("cargo test").into_exact_workspace_allow("/workspace"); + let engine = engine_with_ask_rule(rule); + + let exact = engine.check(ctx("cargo test", OnRequest)).unwrap(); + assert!(!exact.requires_approval); + assert_eq!(exact.matched_action, Some(PermissionAction::Allow)); + + let extra_args = engine + .check(ctx("cargo test --workspace", OnRequest)) + .unwrap(); + assert!( + extra_args.requires_approval, + "an exact remembered grant must not authorize extra arguments" + ); + + let other_repo = engine + .check(ExecPolicyContext { + command: "cargo test", + cwd: "/other", + tool: Some("exec_shell"), + path: None, + ask_for_approval: OnRequest, + sandbox_mode: Some("workspace-write"), + }) + .unwrap(); + assert!( + other_repo.requires_approval, + "a remembered grant must not escape its repository" + ); + } + + #[test] + fn exact_workspace_file_allow_matches_relative_and_absolute_paths_in_repo() { + let rule = ToolAskRule::file_path("write_file", "src/lib.rs") + .into_exact_workspace_allow("/workspace"); + let engine = engine_with_ask_rule(rule); + + for path in ["src/lib.rs", "/workspace/src/lib.rs"] { + let decision = engine + .check(file_ctx("write_file", path, "/workspace", OnRequest)) + .unwrap(); + assert_eq!( + decision.matched_action, + Some(PermissionAction::Allow), + "{path}" + ); + assert!(!decision.requires_approval, "{path}"); + } + + let other_repo = engine + .check(file_ctx("write_file", "src/lib.rs", "/other", OnRequest)) + .unwrap(); + assert!(other_repo.requires_approval); + } + + #[test] + fn exact_workspace_file_allow_preserves_posix_case_boundaries() { + let rule = ToolAskRule::file_path("write_file", "src/Foo.rs") + .into_exact_workspace_allow("/Workspace"); + let engine = engine_with_ask_rule(rule); + + let exact = engine + .check(file_ctx( + "write_file", + "/Workspace/src/Foo.rs", + "/Workspace", + OnRequest, + )) + .unwrap(); + assert_eq!(exact.matched_action, Some(PermissionAction::Allow)); + + for path in ["src/foo.rs", "/workspace/src/Foo.rs"] { + let decision = engine + .check(file_ctx("write_file", path, "/Workspace", OnRequest)) + .unwrap(); + assert!( + decision.requires_approval, + "{path:?} must not inherit a case-distinct grant" + ); + } + } + + #[test] + fn workspace_scope_normalizes_windows_separators_and_case() { + let rule = + ToolAskRule::exec_shell("cargo test").into_exact_workspace_allow(r"C:\Repo\CodeWhale"); + let engine = engine_with_ask_rule(rule); + let decision = engine + .check(ExecPolicyContext { + command: "cargo test", + cwd: "c:/repo/codewhale", + tool: Some("exec_shell"), + path: None, + ask_for_approval: OnRequest, + sandbox_mode: Some("workspace-write"), + }) + .unwrap(); + + assert_eq!(decision.matched_action, Some(PermissionAction::Allow)); + assert_eq!( + normalize_workspace_scope(r"C:\Repo\CodeWhale"), + Some("c:/repo/codewhale".to_string()) + ); + assert_eq!(normalize_workspace_scope("relative/repo"), None); + assert_eq!(normalize_workspace_scope("/"), None); + } + + #[test] + fn workspace_scope_preserves_posix_case_and_rejects_traversal() { + assert_eq!( + normalize_workspace_scope("/Workspace/CodeWhale"), + Some("/Workspace/CodeWhale".to_string()) + ); + assert_ne!( + normalize_workspace_scope("/Workspace/CodeWhale"), + normalize_workspace_scope("/workspace/codewhale") + ); + assert_eq!(normalize_workspace_scope("/workspace/../other"), None); + } + // ── helpers ─────────────────────────────────────────────────────────── fn engine_with_ask_rule(rule: ToolAskRule) -> ExecPolicyEngine { @@ -1976,6 +2221,7 @@ mod tests { command: None, path: None, action, + ..ToolAskRule::new("") } } @@ -1985,6 +2231,7 @@ mod tests { command: None, path: Some(path.to_string()), action, + ..ToolAskRule::new("") } } diff --git a/crates/tui/CHANGELOG.md b/crates/tui/CHANGELOG.md index b70c98d29c..da975454f8 100644 --- a/crates/tui/CHANGELOG.md +++ b/crates/tui/CHANGELOG.md @@ -9,6 +9,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Approval cards can now remember eligible safe shell and file-write approvals + as exact `allow` rules scoped to the current repository. Remembered shell + commands use complete-command matching, validated file and patch paths remain + workspace-relative, and dangerous, critical, or repo-law-held requests stay + ineligible and continue to require review. + +## [0.9.1] - 2026-07-24 + +### Dogfood follow-ups (2026-07-24) + +### Added + - `/compact [focus]`: the manual compaction command now accepts an optional focus argument that is injected into the summary prompt, and the compaction summary itself becomes a structured nine-section @@ -80,7 +92,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 unknown role tokens still fail closed with the canonical vocabulary in the error. -## [0.9.1] - 2026-07-22 The Codewhale v0.9.1 source candidate includes a first-class local web client over the Runtime API, first-class OpenCode Go and TelecomJS TokenHub providers and restored xAI device login, diff --git a/crates/tui/locales/en.json b/crates/tui/locales/en.json index a4c706bc99..36024e1c97 100644 --- a/crates/tui/locales/en.json +++ b/crates/tui/locales/en.json @@ -670,8 +670,10 @@ "ApprovalFieldAbout": "About: ", "ApprovalFieldImpact": "Impact: ", "ApprovalFieldParams": "Params: ", - "ApprovalOptionApproveOnce": "Approve once", - "ApprovalOptionApproveAlways": "Approve for this session (this kind)", + "ApprovalOptionApproveOnce": "Allow once", + "ApprovalOptionApproveAlways": "Allow for this session (this kind)", + "ApprovalOptionAllowExactRepo": "Always allow this exact rule in this repo", + "ApprovalSaveAskRuleHint": " s allow once + always ask exact rule", "ApprovalOptionDeny": "Deny this call", "ApprovalOptionAbortTurn": "Abort the turn", "ApprovalBlockTitle": "approval", diff --git a/crates/tui/locales/es-419.json b/crates/tui/locales/es-419.json index 409874eefa..a2b2526a1a 100644 --- a/crates/tui/locales/es-419.json +++ b/crates/tui/locales/es-419.json @@ -668,8 +668,10 @@ "ApprovalFieldAbout": "Acerca de:", "ApprovalFieldImpact": "Impacto:", "ApprovalFieldParams": "Parámetros:", - "ApprovalOptionApproveOnce": "Aprobar una vez", - "ApprovalOptionApproveAlways": "Aprobar en esta sesión (este tipo)", + "ApprovalOptionApproveOnce": "Permitir una vez", + "ApprovalOptionApproveAlways": "Permitir durante esta sesión (este tipo)", + "ApprovalOptionAllowExactRepo": "Permitir siempre esta regla exacta en este repositorio", + "ApprovalSaveAskRuleHint": " s permitir una vez + preguntar siempre por la regla exacta", "ApprovalOptionDeny": "Denegar esta llamada", "ApprovalOptionAbortTurn": "Abortar turno", "ApprovalBlockTitle": "aprobación", diff --git a/crates/tui/locales/ja.json b/crates/tui/locales/ja.json index 42bcf031e4..da33f36245 100644 --- a/crates/tui/locales/ja.json +++ b/crates/tui/locales/ja.json @@ -668,8 +668,10 @@ "ApprovalFieldAbout": "詳細：", "ApprovalFieldImpact": "影響：", "ApprovalFieldParams": "パラメータ：", - "ApprovalOptionApproveOnce": "1回だけ承認", - "ApprovalOptionApproveAlways": "このセッションで承認（この種類）", + "ApprovalOptionApproveOnce": "今回のみ許可", + "ApprovalOptionApproveAlways": "このセッションで許可（同種）", + "ApprovalOptionAllowExactRepo": "このリポジトリでこの完全一致ルールを常に許可", + "ApprovalSaveAskRuleHint": " s 今回のみ許可 + 完全一致ルールを常に確認", "ApprovalOptionDeny": "拒否", "ApprovalOptionAbortTurn": "中断", "ApprovalBlockTitle": "承認", diff --git a/crates/tui/locales/ko.json b/crates/tui/locales/ko.json index 7662988f70..9de306a4d4 100644 --- a/crates/tui/locales/ko.json +++ b/crates/tui/locales/ko.json @@ -670,8 +670,10 @@ "ApprovalFieldAbout": "설명: ", "ApprovalFieldImpact": "영향: ", "ApprovalFieldParams": "매개변수: ", - "ApprovalOptionApproveOnce": "한 번 승인", - "ApprovalOptionApproveAlways": "이 세션에서 항상 승인 (이 종류)", + "ApprovalOptionApproveOnce": "한 번 허용", + "ApprovalOptionApproveAlways": "이 세션에서 허용 (이 종류)", + "ApprovalOptionAllowExactRepo": "이 저장소에서 이 정확한 규칙을 항상 허용", + "ApprovalSaveAskRuleHint": " s 한 번 허용 + 정확한 규칙은 항상 묻기", "ApprovalOptionDeny": "이 호출 거부", "ApprovalOptionAbortTurn": "턴 중단", "ApprovalBlockTitle": "승인", diff --git a/crates/tui/locales/pt-BR.json b/crates/tui/locales/pt-BR.json index 2c940f3db0..0bd7066cb4 100644 --- a/crates/tui/locales/pt-BR.json +++ b/crates/tui/locales/pt-BR.json @@ -668,8 +668,10 @@ "ApprovalFieldAbout": "Sobre:", "ApprovalFieldImpact": "Impacto:", "ApprovalFieldParams": "Parâmetros:", - "ApprovalOptionApproveOnce": "Aprovar uma vez", - "ApprovalOptionApproveAlways": "Aprovar nesta sessão (este tipo)", + "ApprovalOptionApproveOnce": "Permitir uma vez", + "ApprovalOptionApproveAlways": "Permitir nesta sessão (este tipo)", + "ApprovalOptionAllowExactRepo": "Sempre permitir esta regra exata neste repositório", + "ApprovalSaveAskRuleHint": " s permitir uma vez + sempre perguntar pela regra exata", "ApprovalOptionDeny": "Negar esta chamada", "ApprovalOptionAbortTurn": "Abortar turno", "ApprovalBlockTitle": "aprovação", diff --git a/crates/tui/locales/vi.json b/crates/tui/locales/vi.json index aff67d7434..9fd684f8a1 100644 --- a/crates/tui/locales/vi.json +++ b/crates/tui/locales/vi.json @@ -668,8 +668,10 @@ "ApprovalFieldAbout": "Mô tả:", "ApprovalFieldImpact": "Tác động:", "ApprovalFieldParams": "Tham số:", - "ApprovalOptionApproveOnce": "Phê duyệt một lần", - "ApprovalOptionApproveAlways": "Phê duyệt trong phiên này (loại này)", + "ApprovalOptionApproveOnce": "Cho phép một lần", + "ApprovalOptionApproveAlways": "Cho phép trong phiên này (loại này)", + "ApprovalOptionAllowExactRepo": "Luôn cho phép quy tắc chính xác này trong kho mã", + "ApprovalSaveAskRuleHint": " s cho phép một lần + luôn hỏi với quy tắc chính xác", "ApprovalOptionDeny": "Từ chối lần gọi này", "ApprovalOptionAbortTurn": "Hủy bỏ lượt", "ApprovalBlockTitle": "phê duyệt", diff --git a/crates/tui/locales/zh-Hans.json b/crates/tui/locales/zh-Hans.json index c1ff2c31c3..3227cd23e2 100644 --- a/crates/tui/locales/zh-Hans.json +++ b/crates/tui/locales/zh-Hans.json @@ -668,8 +668,10 @@ "ApprovalFieldAbout": "说明：", "ApprovalFieldImpact": "影响：", "ApprovalFieldParams": "参数：", - "ApprovalOptionApproveOnce": "仅本次批准", - "ApprovalOptionApproveAlways": "本会话同类自动批准", + "ApprovalOptionApproveOnce": "仅允许本次", + "ApprovalOptionApproveAlways": "本会话允许同类操作", + "ApprovalOptionAllowExactRepo": "在此仓库中始终允许这条精确规则", + "ApprovalSaveAskRuleHint": " s 仅允许本次并始终询问精确规则", "ApprovalOptionDeny": "拒绝本次调用", "ApprovalOptionAbortTurn": "终止本轮", "ApprovalBlockTitle": "审批", diff --git a/crates/tui/locales/zh-Hant.json b/crates/tui/locales/zh-Hant.json index 93404c8fc8..80da3d2843 100644 --- a/crates/tui/locales/zh-Hant.json +++ b/crates/tui/locales/zh-Hant.json @@ -39,8 +39,10 @@ "ApprovalFieldAbout": "說明：", "ApprovalFieldImpact": "影響：", "ApprovalFieldParams": "參數：", - "ApprovalOptionApproveOnce": "僅批准一次", - "ApprovalOptionApproveAlways": "本會話同類自動批准", + "ApprovalOptionApproveOnce": "僅允許一次", + "ApprovalOptionApproveAlways": "在此工作階段允許（此類）", + "ApprovalOptionAllowExactRepo": "在此儲存庫中一律允許這條精確規則", + "ApprovalSaveAskRuleHint": " s 僅允許一次 + 一律詢問精確規則", "ApprovalOptionDeny": "拒絕本次調用", "ApprovalOptionAbortTurn": "終止本輪", "ApprovalBlockTitle": "審批", diff --git a/crates/tui/src/core/engine.rs b/crates/tui/src/core/engine.rs index b100e9c949..9e66f84953 100644 --- a/crates/tui/src/core/engine.rs +++ b/crates/tui/src/core/engine.rs @@ -1291,15 +1291,22 @@ impl Engine { &self.session.workspace, self.session.approval_mode, ); - if let Some(ToolAskRuleDecision::Prompt(reason)) = ask_rule_decision.as_ref() { - // YOLO mode (auto_approve) is the explicit "no approvals" - // contract: a typed ask-rule must not pop a modal in YOLO. - // A typed deny rule still blocks hard below. - if !self.session.auto_approve { - approval_required = true; - approval_description = reason.clone(); - approval_force_prompt = true; + match ask_rule_decision.as_ref() { + Some(ToolAskRuleDecision::Allow) => { + approval_required = false; + approval_force_prompt = false; } + Some(ToolAskRuleDecision::Prompt(reason)) => { + // YOLO mode (auto_approve) is the explicit "no approvals" + // contract: a typed ask-rule must not pop a modal in YOLO. + // A typed deny rule still blocks hard below. + if !self.session.auto_approve { + approval_required = true; + approval_description = reason.clone(); + approval_force_prompt = true; + } + } + Some(ToolAskRuleDecision::Block(_)) | None => {} } if let Some(ToolAskRuleDecision::Block(reason)) = ask_rule_decision { Err(ToolError::permission_denied(reason)) @@ -4555,6 +4562,7 @@ fn goal_objective_for_prompt( #[derive(Debug, Clone, PartialEq, Eq)] pub(super) enum ToolAskRuleDecision { + Allow, Prompt(String), Block(String), } @@ -4680,6 +4688,7 @@ pub(super) fn file_tool_ask_rule_decision( } let mut prompt: Option = None; + let mut all_allowed = true; for path in paths { match tool_ask_rule_decision_for_context( config, @@ -4694,11 +4703,19 @@ pub(super) fn file_tool_ask_rule_decision( } Some(ToolAskRuleDecision::Prompt(reason)) => { prompt.get_or_insert(reason); + all_allowed = false; } - None => {} + Some(ToolAskRuleDecision::Allow) => {} + None => all_allowed = false, } } - prompt.map(ToolAskRuleDecision::Prompt) + if let Some(prompt) = prompt { + Some(ToolAskRuleDecision::Prompt(prompt)) + } else if all_allowed { + Some(ToolAskRuleDecision::Allow) + } else { + None + } } fn tool_ask_rule_decision_for_context( @@ -4731,6 +4748,8 @@ fn tool_ask_rule_decision_for_context( Some(ToolAskRuleDecision::Block(decision.reason().to_string())) } else if decision.requires_approval { Some(ToolAskRuleDecision::Prompt(decision.reason().to_string())) + } else if decision.matched_action == Some(codewhale_execpolicy::PermissionAction::Allow) { + Some(ToolAskRuleDecision::Allow) } else { None } diff --git a/crates/tui/src/core/engine/tests.rs b/crates/tui/src/core/engine/tests.rs index b90de3770d..5d5b46ede0 100644 --- a/crates/tui/src/core/engine/tests.rs +++ b/crates/tui/src/core/engine/tests.rs @@ -4082,6 +4082,49 @@ fn exec_shell_ask_rule_decision_ignores_unmatched_command() { assert_eq!(decision, None); } +#[test] +fn exec_shell_allow_rule_decision_allows_only_exact_command_in_scoped_repo() { + let rule = codewhale_execpolicy::ToolAskRule::exec_shell("cargo test") + .into_exact_workspace_allow("/repo"); + let config = EngineConfig { + exec_policy_engine: codewhale_execpolicy::ExecPolicyEngine::with_rulesets(vec![ + codewhale_execpolicy::Ruleset::user(vec![], vec![]).with_ask_rules(vec![rule]), + ]), + ..EngineConfig::default() + }; + + assert_eq!( + exec_shell_ask_rule_decision( + &config, + "exec_shell", + &json!({"command": "cargo test"}), + Path::new("/repo"), + crate::tui::approval::ApprovalMode::Suggest, + ), + Some(ToolAskRuleDecision::Allow) + ); + assert_eq!( + exec_shell_ask_rule_decision( + &config, + "exec_shell", + &json!({"command": "cargo test --workspace"}), + Path::new("/repo"), + crate::tui::approval::ApprovalMode::Suggest, + ), + None + ); + assert_eq!( + exec_shell_ask_rule_decision( + &config, + "exec_shell", + &json!({"command": "cargo test"}), + Path::new("/other"), + crate::tui::approval::ApprovalMode::Suggest, + ), + None + ); +} + #[test] fn file_ask_rule_decision_prompts_for_matching_read_path() { let config = EngineConfig { @@ -4194,6 +4237,51 @@ fn file_ask_rule_decision_ignores_unmatched_path() { assert_eq!(decision, None); } +#[test] +fn apply_patch_allow_requires_every_touched_path_to_match() { + let rules = ["src/a.rs", "src/b.rs"] + .into_iter() + .map(|path| { + codewhale_execpolicy::ToolAskRule::file_path("apply_patch", path) + .into_exact_workspace_allow("/repo") + }) + .collect(); + let config = EngineConfig { + exec_policy_engine: codewhale_execpolicy::ExecPolicyEngine::with_rulesets(vec![ + codewhale_execpolicy::Ruleset::user(vec![], vec![]).with_ask_rules(rules), + ]), + ..EngineConfig::default() + }; + + let fully_allowed = file_tool_ask_rule_decision( + &config, + "apply_patch", + &json!({ + "replace": [ + {"path": "src/a.rs", "content": "a"}, + {"path": "src/b.rs", "content": "b"} + ] + }), + Path::new("/repo"), + crate::tui::approval::ApprovalMode::Suggest, + ); + assert_eq!(fully_allowed, Some(ToolAskRuleDecision::Allow)); + + let partially_allowed = file_tool_ask_rule_decision( + &config, + "apply_patch", + &json!({ + "replace": [ + {"path": "src/a.rs", "content": "a"}, + {"path": "src/c.rs", "content": "c"} + ] + }), + Path::new("/repo"), + crate::tui::approval::ApprovalMode::Suggest, + ); + assert_eq!(partially_allowed, None); +} + fn api_tool(name: &str) -> Tool { Tool { tool_type: Some("function".to_string()), diff --git a/crates/tui/src/core/engine/turn_loop.rs b/crates/tui/src/core/engine/turn_loop.rs index 438eba667b..6ccf8b7074 100644 --- a/crates/tui/src/core/engine/turn_loop.rs +++ b/crates/tui/src/core/engine/turn_loop.rs @@ -1919,6 +1919,16 @@ impl Engine { }); if let Some(decision) = ask_rule_decision { match decision { + ToolAskRuleDecision::Allow => { + // Remembered grants bypass ordinary registry + // approval only. Hook asks and non-bypassable + // tool requirements remain monotonic, while + // auto-review and repo-law floors below can + // still force review or block. + if !hook_requires_approval && !approval_force_prompt { + approval_required = false; + } + } ToolAskRuleDecision::Prompt(reason) => { // #3790: the mode is the sole authority — a typed // ask-rule prompts in Agent/Plan but never in YOLO diff --git a/crates/tui/src/localization.rs b/crates/tui/src/localization.rs index 1d08ab87ab..7ad24d0318 100644 --- a/crates/tui/src/localization.rs +++ b/crates/tui/src/localization.rs @@ -795,6 +795,8 @@ pub enum MessageId { ApprovalFieldParams, ApprovalOptionApproveOnce, ApprovalOptionApproveAlways, + ApprovalOptionAllowExactRepo, + ApprovalSaveAskRuleHint, ApprovalOptionDeny, ApprovalOptionAbortTurn, ApprovalBlockTitle, @@ -1965,6 +1967,8 @@ pub const ALL_MESSAGE_IDS: &[MessageId] = &[ MessageId::ApprovalFieldParams, MessageId::ApprovalOptionApproveOnce, MessageId::ApprovalOptionApproveAlways, + MessageId::ApprovalOptionAllowExactRepo, + MessageId::ApprovalSaveAskRuleHint, MessageId::ApprovalOptionDeny, MessageId::ApprovalOptionAbortTurn, MessageId::ApprovalBlockTitle, diff --git a/crates/tui/src/tui/approval.rs b/crates/tui/src/tui/approval.rs index 549447389e..8fac89dc9e 100644 --- a/crates/tui/src/tui/approval.rs +++ b/crates/tui/src/tui/approval.rs @@ -34,6 +34,7 @@ use crate::tools::canonical_action::canonical_action_alias; use crate::tui::views::{ModalKind, ModalView, ViewAction, ViewEvent}; use crate::tui::widgets::{ApprovalWidget, ElevationWidget, Renderable}; use codewhale_config::ToolAskRule; +use codewhale_execpolicy::PermissionAction; use crossterm::event::{KeyCode, KeyEvent, MouseButton, MouseEvent, MouseEventKind}; use ratatui::layout::Rect; use serde_json::Value; @@ -149,6 +150,8 @@ pub struct ApprovalRequest { pub intent_summary: Option, /// Ask-only persistent rules that can be saved with the approval. pub persistent_ask_rules: Vec, + /// Exact repo-scoped allow rules available for safe approval requests. + pub persistent_allow_rules: Vec, } /// Key approval details rendered prominently in the approval card. @@ -161,25 +164,32 @@ pub struct ApprovalDetail { pub shell_lines: Option>, } -/// Human-readable preview of ask-only rules the `S` approval shortcut would -/// append. This is intentionally derived from `persistent_ask_rules` only; the -/// approval UI must not re-parse tool inputs such as patches. +/// Human-readable preview of rules an approval action would append. +/// +/// This is intentionally derived from the already validated persistent-rule +/// candidates; the approval UI must not re-parse tool inputs such as patches. #[derive(Debug, Clone, PartialEq, Eq)] -pub struct AskRuleSavePreview { +pub struct PermissionRuleSavePreview { + pub action: PermissionAction, pub rule_count: usize, pub entries: Vec, pub omitted: usize, } -impl AskRuleSavePreview { +impl PermissionRuleSavePreview { #[must_use] pub fn summary(&self) -> String { + let action = match self.action { + PermissionAction::Allow => "allow", + PermissionAction::Ask => "ask", + PermissionAction::Deny => "deny", + }; let noun = if self.rule_count == 1 { "rule" } else { "rules" }; - format!("{} ask {noun}", self.rule_count) + format!("{} {action} {noun}", self.rule_count) } } @@ -191,8 +201,7 @@ impl ApprovalRequest { /// `.codewhale/constitution.json` ask rule forces review. #[must_use] pub fn is_repo_law_prompt(&self) -> bool { - self.description.starts_with("Repo law holds this write:") - && self.description.contains(".codewhale/constitution.json") + description_is_repo_law_prompt(&self.description) } /// Presentation stakes for this request (see [`ApprovalStakes`]). @@ -234,6 +243,21 @@ impl ApprovalRequest { let risk = classify_risk(tool_name, category, params); let approval_grouping_key = crate::tools::approval_cache::build_approval_grouping_key(tool_name, params).0; + let persistent_ask_rules = + build_persistent_ask_rules(semantic_tool_name, params, workspace); + let persistent_allow_rules = if classify_stakes(tool_name, category, risk, params) + == ApprovalStakes::Critical + || description_is_repo_law_prompt(description) + { + Vec::new() + } else { + build_persistent_allow_rules( + semantic_tool_name, + params, + workspace, + &persistent_ask_rules, + ) + }; Self { id: id.to_string(), @@ -253,7 +277,8 @@ impl ApprovalRequest { Some(summary.to_string()) } }), - persistent_ask_rules: build_persistent_ask_rules(semantic_tool_name, params, workspace), + persistent_ask_rules, + persistent_allow_rules, } } @@ -289,13 +314,31 @@ impl ApprovalRequest { } #[must_use] - pub fn ask_rule_save_preview(&self) -> Option { - build_ask_rule_save_preview( + pub fn can_save_allow_rule(&self) -> bool { + !self.persistent_allow_rules.is_empty() + && self.stakes() != ApprovalStakes::Critical + && !self.is_repo_law_prompt() + } + + #[must_use] + pub fn ask_rule_save_preview(&self) -> Option { + build_permission_rule_save_preview( &self.persistent_ask_rules, ASK_RULE_SAVE_PREVIEW_MAX_ENTRIES, ) } + #[must_use] + pub fn allow_rule_save_preview(&self) -> Option { + self.can_save_allow_rule().then(|| { + build_permission_rule_save_preview( + &self.persistent_allow_rules, + ASK_RULE_SAVE_PREVIEW_MAX_ENTRIES, + ) + .expect("eligible allow rules are non-empty") + }) + } + #[must_use] #[cfg(test)] pub fn ask_rule_preview(&self) -> Option { @@ -330,11 +373,16 @@ impl ApprovalRequest { } } +fn description_is_repo_law_prompt(description: &str) -> bool { + description.starts_with("Repo law holds this write:") + && description.contains(".codewhale/constitution.json") +} + #[must_use] -fn build_ask_rule_save_preview( +fn build_permission_rule_save_preview( rules: &[ToolAskRule], max_entries: usize, -) -> Option { +) -> Option { if rules.is_empty() { return None; } @@ -342,9 +390,10 @@ fn build_ask_rule_save_preview( let entries = rules .iter() .take(max_entries) - .map(format_ask_rule_save_entry) + .map(format_permission_rule_save_entry) .collect(); - Some(AskRuleSavePreview { + Some(PermissionRuleSavePreview { + action: rules[0].action, rule_count: rules.len(), entries, omitted: rules.len().saturating_sub(max_entries), @@ -352,7 +401,7 @@ fn build_ask_rule_save_preview( } #[must_use] -fn format_ask_rule_save_entry(rule: &ToolAskRule) -> String { +fn format_permission_rule_save_entry(rule: &ToolAskRule) -> String { let mut parts = vec![format!( "tool={}", sanitize_ask_rule_preview_value(&rule.tool) @@ -366,6 +415,15 @@ fn format_ask_rule_save_entry(rule: &ToolAskRule) -> String { if let Some(path) = &rule.path { parts.push(format!("path={}", sanitize_ask_rule_preview_value(path))); } + if rule.command_exact { + parts.push("command_exact=true".to_string()); + } + if let Some(workspace) = &rule.workspace { + parts.push(format!( + "workspace={}", + sanitize_ask_rule_preview_value(workspace) + )); + } parts.join(" ") } @@ -395,6 +453,43 @@ fn build_persistent_ask_rules( } } +#[must_use] +fn build_persistent_allow_rules( + tool_name: &str, + params: &Value, + workspace: &Path, + exact_rules: &[ToolAskRule], +) -> Vec { + if exact_rules.is_empty() { + return Vec::new(); + } + + if tool_name == "exec_shell" { + let Some(command) = params.get("command").and_then(Value::as_str) else { + return Vec::new(); + }; + if !matches!( + crate::command_safety::analyze_command(command).level, + crate::command_safety::SafetyLevel::Safe + | crate::command_safety::SafetyLevel::WorkspaceSafe + ) { + return Vec::new(); + } + } + + let workspace = workspace.to_string_lossy(); + let Some(workspace) = codewhale_execpolicy::normalize_workspace_scope(workspace.as_ref()) + else { + return Vec::new(); + }; + + exact_rules + .iter() + .cloned() + .map(|rule| rule.into_exact_workspace_allow(workspace.clone())) + .collect() +} + #[must_use] fn build_exec_shell_ask_rules(params: &Value) -> Vec { let Some(command) = params @@ -1236,6 +1331,7 @@ fn split_unquoted_redirect(command: &str) -> Option<(&str, &str)> { pub enum ApprovalOption { ApproveOnce, ApproveAlways, + AllowExactRepo, Deny, Abort, } @@ -1247,6 +1343,13 @@ impl ApprovalOption { ApprovalOption::Deny, ApprovalOption::Abort, ]; + const ORDER_WITH_PERSISTENT_ALLOW: [ApprovalOption; 5] = [ + ApprovalOption::ApproveOnce, + ApprovalOption::ApproveAlways, + ApprovalOption::AllowExactRepo, + ApprovalOption::Deny, + ApprovalOption::Abort, + ]; /// Workflow elevated-plan card (#4126): Approve / Edit plan / Cancel. const WORKFLOW_ORDER: [ApprovalOption; 3] = [ @@ -1255,32 +1358,35 @@ impl ApprovalOption { ApprovalOption::Abort, ]; - fn order_for(tool_name: &str) -> &'static [ApprovalOption] { - if tool_name == "workflow" { + fn order_for(request: &ApprovalRequest) -> &'static [ApprovalOption] { + if request.tool_name == "workflow" { &Self::WORKFLOW_ORDER + } else if request.can_save_allow_rule() { + &Self::ORDER_WITH_PERSISTENT_ALLOW } else { &Self::ORDER } } - fn from_index_for(tool_name: &str, idx: usize) -> ApprovalOption { - Self::order_for(tool_name) + fn from_index_for(request: &ApprovalRequest, idx: usize) -> ApprovalOption { + Self::order_for(request) .get(idx) .copied() .unwrap_or(Self::Abort) } - fn index_for(self, tool_name: &str) -> usize { - Self::order_for(tool_name) + fn index_for(self, request: &ApprovalRequest) -> usize { + Self::order_for(request) .iter() .position(|o| *o == self) - .unwrap_or(Self::order_for(tool_name).len().saturating_sub(1)) + .unwrap_or(Self::order_for(request).len().saturating_sub(1)) } fn decision(self) -> ReviewDecision { match self { ApprovalOption::ApproveOnce => ReviewDecision::Approved, ApprovalOption::ApproveAlways => ReviewDecision::ApprovedForSession, + ApprovalOption::AllowExactRepo => ReviewDecision::Approved, // Workflow maps Deny → "Edit plan" (model revises plan). ApprovalOption::Deny => ReviewDecision::Denied, ApprovalOption::Abort => ReviewDecision::Abort, @@ -1324,14 +1430,14 @@ impl ApprovalView { } fn select_next(&mut self) { - let max = ApprovalOption::order_for(&self.request.tool_name) + let max = ApprovalOption::order_for(&self.request) .len() .saturating_sub(1); self.selected = (self.selected + 1).min(max); } fn current_option(&self) -> ApprovalOption { - ApprovalOption::from_index_for(&self.request.tool_name, self.selected) + ApprovalOption::from_index_for(&self.request, self.selected) } /// Whether this approval is the elevated Workflow plan card (#4126). @@ -1367,8 +1473,16 @@ impl ApprovalView { /// Commit the given option and close the approval modal. fn commit_option(&mut self, option: ApprovalOption) -> ViewAction { - self.selected = option.index_for(&self.request.tool_name); - self.emit_decision(option.decision(), false) + self.selected = option.index_for(&self.request); + if option == ApprovalOption::AllowExactRepo && self.request.can_save_allow_rule() { + self.emit_decision_with_rules( + option.decision(), + false, + self.request.persistent_allow_rules.clone(), + ) + } else { + self.emit_decision(option.decision(), false) + } } fn emit_decision(&self, decision: ReviewDecision, timed_out: bool) -> ViewAction { @@ -1379,7 +1493,7 @@ impl ApprovalView { &self, decision: ReviewDecision, timed_out: bool, - persistent_ask_rules: Vec, + persistent_rules: Vec, ) -> ViewAction { ViewAction::EmitAndClose(ViewEvent::ApprovalDecision { tool_id: self.request.id.clone(), @@ -1388,7 +1502,7 @@ impl ApprovalView { timed_out, approval_key: self.request.approval_key.clone(), approval_grouping_key: self.request.approval_grouping_key.clone(), - persistent_ask_rules, + persistent_rules, }) } @@ -1471,6 +1585,9 @@ impl ModalView for ApprovalView { { self.commit_option(ApprovalOption::ApproveAlways) } + KeyCode::Char('p') | KeyCode::Char('P') if self.request.can_save_allow_rule() => { + self.commit_option(ApprovalOption::AllowExactRepo) + } // Workflow plan card (#4126): [2/e] Edit plan, [3/n/d] Cancel. KeyCode::Char('e') | KeyCode::Char('E') | KeyCode::Char('2') if self.is_workflow_plan_approval() => @@ -1519,10 +1636,8 @@ impl ModalView for ApprovalView { rect.contains(ratatui::layout::Position::new(mouse.column, mouse.row)) }); if let Some(index) = clicked { - return self.commit_option(ApprovalOption::from_index_for( - &self.request.tool_name, - index, - )); + return self + .commit_option(ApprovalOption::from_index_for(&self.request, index)); } ViewAction::None } @@ -2503,6 +2618,48 @@ mod tests { assert_eq!(preview.omitted, 0); } + #[test] + fn safe_shell_request_builds_exact_workspace_allow_rule() { + let request = shell_request(); + let expected = ToolAskRule::exec_shell("cargo test --workspace") + .into_exact_workspace_allow("/workspace"); + + assert!(request.can_save_allow_rule()); + assert_eq!(request.persistent_allow_rules, vec![expected]); + let preview = request.allow_rule_save_preview().expect("allow preview"); + assert_eq!(preview.summary(), "1 allow rule"); + assert_eq!( + preview.entries, + vec![ + "tool=exec_shell command=cargo test --workspace command_exact=true workspace=/workspace" + ] + ); + } + + #[test] + fn unsafe_shell_requests_cannot_persist_allow_rules() { + for command in [ + "rm -rf ~/", + "git push origin main", + "curl https://example.com", + "cargo test && git status", + ] { + let request = ApprovalRequest::new( + "test-id", + "exec_shell", + "Run a shell command", + &json!({"command": command}), + "tool:exec_shell", + ); + assert!( + request.persistent_allow_rules.is_empty(), + "{command:?} must not produce a remembered allow grant" + ); + assert!(!request.can_save_allow_rule(), "{command:?}"); + assert_eq!(request.allow_rule_save_preview(), None, "{command:?}"); + } + } + #[test] fn file_ask_rule_saved_for_write_file_approval() { // A write_file approval offers an exact, workspace-relative file rule @@ -2520,6 +2677,23 @@ mod tests { assert!(preview.contains("path = \"src/main.rs\"")); } + #[test] + fn file_write_builds_exact_workspace_allow_rule() { + let request = destructive_request(); + let expected = ToolAskRule::file_path("write_file", "src/main.rs") + .into_exact_workspace_allow("/workspace"); + + assert!(request.can_save_allow_rule()); + assert_eq!(request.persistent_allow_rules, vec![expected]); + assert_eq!( + request + .allow_rule_save_preview() + .expect("allow preview") + .entries, + vec!["tool=write_file path=src/main.rs workspace=/workspace"] + ); + } + #[test] fn ask_rule_save_preview_formats_write_and_edit_file_paths() { let write = destructive_request(); @@ -2639,6 +2813,15 @@ diff --git a/src/b.rs b/src/b.rs "tool=apply_patch path=src/b.rs" ] ); + assert_eq!( + request.persistent_allow_rules, + vec![ + ToolAskRule::file_path("apply_patch", "src/a.rs") + .into_exact_workspace_allow("/workspace"), + ToolAskRule::file_path("apply_patch", "src/b.rs") + .into_exact_workspace_allow("/workspace"), + ] + ); } #[test] @@ -2754,7 +2937,7 @@ diff --git a/src/b.rs b/src/b.rs ToolAskRule::file_path("apply_patch", "src/d.rs"), ]; - let preview = build_ask_rule_save_preview(&rules, 2).expect("save preview"); + let preview = build_permission_rule_save_preview(&rules, 2).expect("save preview"); assert_eq!(preview.rule_count, 4); assert_eq!(preview.summary(), "4 ask rules"); assert_eq!( @@ -2833,7 +3016,7 @@ diff --git a/src/b.rs b/src/b.rs let action = view.handle_key(create_key_event(KeyCode::Char('s'))); let ViewAction::EmitAndClose(ViewEvent::ApprovalDecision { decision, - persistent_ask_rules, + persistent_rules, .. }) = action else { @@ -2842,7 +3025,7 @@ diff --git a/src/b.rs b/src/b.rs assert_eq!(decision, ReviewDecision::Approved); assert_eq!( - persistent_ask_rules, + persistent_rules, vec![ToolAskRule::exec_shell("cargo test --workspace")] ); } @@ -2856,7 +3039,7 @@ diff --git a/src/b.rs b/src/b.rs let action = view.handle_key(create_key_event(KeyCode::Char('S'))); let ViewAction::EmitAndClose(ViewEvent::ApprovalDecision { decision, - persistent_ask_rules, + persistent_rules, .. }) = action else { @@ -2865,11 +3048,63 @@ diff --git a/src/b.rs b/src/b.rs assert_eq!(decision, ReviewDecision::Approved); assert_eq!( - persistent_ask_rules, + persistent_rules, vec![ToolAskRule::file_path("write_file", "src/main.rs")] ); } + #[test] + fn persistent_allow_option_approves_once_with_exact_repo_rule() { + let mut view = ApprovalView::new(shell_request()); + + let action = view.handle_key(create_key_event(KeyCode::Char('p'))); + let ViewAction::EmitAndClose(ViewEvent::ApprovalDecision { + decision, + persistent_rules, + .. + }) = action + else { + panic!("expected approval decision"); + }; + + assert_eq!(decision, ReviewDecision::Approved); + assert_eq!( + persistent_rules, + vec![ + ToolAskRule::exec_shell("cargo test --workspace") + .into_exact_workspace_allow("/workspace") + ] + ); + } + + #[test] + fn persistent_allow_shortcut_is_ignored_for_dangerous_command() { + let request = critical_request(); + assert!(request.persistent_allow_rules.is_empty()); + let mut view = ApprovalView::new(request); + + assert!(matches!( + view.handle_key(create_key_event(KeyCode::Char('p'))), + ViewAction::None + )); + } + + #[test] + fn repo_law_request_does_not_build_a_persistent_allow_candidate() { + let request = ApprovalRequest::new( + "test-id", + "edit_file", + "Repo law holds this write: protected path (matched Cargo.toml, .codewhale/constitution.json)", + &json!({"path": "Cargo.toml", "old": "a", "new": "b"}), + "tool:edit_file", + ); + + assert!(request.is_repo_law_prompt()); + assert!(request.persistent_allow_rules.is_empty()); + assert!(!request.can_save_allow_rule()); + assert_eq!(request.allow_rule_save_preview(), None); + } + #[test] fn save_ask_rule_shortcut_is_ignored_without_rule() { let mut view = ApprovalView::new(benign_request()); @@ -2935,6 +3170,7 @@ diff --git a/src/b.rs b/src/b.rs let expected = [ ReviewDecision::Approved, ReviewDecision::ApprovedForSession, + ReviewDecision::Approved, ReviewDecision::Denied, ReviewDecision::Abort, ]; @@ -3557,7 +3793,7 @@ diff --git a/src/b.rs b/src/b.rs "impact dossier is critical-only:\n{joined}" ); assert!( - joined.contains("仅本次批准"), + joined.contains("仅允许本次"), "missing zh approve option:\n{joined}" ); } @@ -3605,7 +3841,7 @@ diff --git a/src/b.rs b/src/b.rs "missing zh policy semantics:\n{joined}" ); assert!( - joined.contains("仅本次批准"), + joined.contains("仅允许本次"), "missing zh approve option:\n{joined}" ); } diff --git a/crates/tui/src/tui/ui.rs b/crates/tui/src/tui/ui.rs index 2fa3ad26e5..335948e95c 100644 --- a/crates/tui/src/tui/ui.rs +++ b/crates/tui/src/tui/ui.rs @@ -13049,7 +13049,7 @@ async fn handle_view_events( timed_out, approval_key, approval_grouping_key, - persistent_ask_rules, + persistent_rules, } => { apply_approval_decision( app, @@ -13062,7 +13062,7 @@ async fn handle_view_events( timed_out, approval_key, approval_grouping_key, - persistent_ask_rules, + persistent_rules, }, ) .await; @@ -14012,7 +14012,7 @@ struct ApprovalDecisionEvent { timed_out: bool, approval_key: String, approval_grouping_key: String, - persistent_ask_rules: Vec, + persistent_rules: Vec, } async fn apply_approval_decision( @@ -14034,10 +14034,10 @@ async fn apply_approval_decision( if matches!( event.decision, ReviewDecision::Approved | ReviewDecision::ApprovedForSession - ) && !event.persistent_ask_rules.is_empty() + ) && !event.persistent_rules.is_empty() && !event.timed_out { - persist_ask_rules_from_approval(app, config, &event.persistent_ask_rules); + persist_rules_from_approval(app, config, &event.persistent_rules); } match event.decision { @@ -14235,31 +14235,49 @@ fn apply_setup_runtime_preset( Ok(format!("Applied {}.", preset.result_summary())) } -fn persist_ask_rules_from_approval( +fn persist_rules_from_approval( app: &mut App, config: &mut Config, rules: &[codewhale_config::ToolAskRule], ) { + let action = rules.first().map(|rule| rule.action); match codewhale_config::ConfigStore::load(app.config_path.clone()).and_then(|mut store| { - let added = store.append_ask_rules(rules)?; + let added = match action { + Some(codewhale_execpolicy::PermissionAction::Ask) => store.append_ask_rules(rules)?, + Some(codewhale_execpolicy::PermissionAction::Allow) => { + store.append_allow_rules(rules)? + } + Some(codewhale_execpolicy::PermissionAction::Deny) => { + anyhow::bail!("the approval UI cannot persist deny rules") + } + None => 0, + }; let permissions_path = store.permissions_path(); config.exec_policy_engine = store.exec_policy_engine(); Ok((added, permissions_path)) }) { Ok((added, path)) if added > 0 => { + let action = match action { + Some(codewhale_execpolicy::PermissionAction::Allow) => "allow", + _ => "ask", + }; app.status_message = Some(format!( - "Saved {added} ask permission rule(s) to {}", + "Saved {added} {action} permission rule(s) to {}", path.display() )); } Ok((_added, path)) => { + let action = match action { + Some(codewhale_execpolicy::PermissionAction::Allow) => "Allow", + _ => "Ask", + }; app.status_message = Some(format!( - "Ask permission rule already saved in {}", + "{action} permission rule already saved in {}", path.display() )); } Err(err) => { - app.status_message = Some(format!("Failed to save ask permission rule: {err:#}")); + app.status_message = Some(format!("Failed to save permission rule: {err:#}")); } } } diff --git a/crates/tui/src/tui/ui/tests.rs b/crates/tui/src/tui/ui/tests.rs index cbdda9a985..2b8fd3099e 100644 --- a/crates/tui/src/tui/ui/tests.rs +++ b/crates/tui/src/tui/ui/tests.rs @@ -15282,7 +15282,7 @@ async fn approval_decision_persists_ask_rules_to_permissions_file() { timed_out: false, approval_key: "approval-key".to_string(), approval_grouping_key: "approval-group".to_string(), - persistent_ask_rules: vec![rule.clone()], + persistent_rules: vec![rule.clone()], }, ) .await; @@ -15315,6 +15315,79 @@ async fn approval_decision_persists_ask_rules_to_permissions_file() { assert!(decision.requires_approval); } +#[tokio::test] +async fn approval_decision_persists_exact_workspace_allow_rule() { + let tmp = TempDir::new().expect("tempdir"); + let config_path = tmp.path().join("config.toml"); + let mut app = create_test_app(); + app.workspace = tmp.path().to_path_buf(); + app.config_path = Some(config_path.clone()); + let mut config = Config::default(); + let mut engine = mock_engine_handle(); + let rule = codewhale_config::ToolAskRule::exec_shell("cargo test") + .into_exact_workspace_allow(tmp.path().to_string_lossy()); + + apply_approval_decision( + &mut app, + &mut engine.handle, + &mut config, + ApprovalDecisionEvent { + tool_id: "tool-allow".to_string(), + tool_name: "exec_shell".to_string(), + decision: ReviewDecision::Approved, + timed_out: false, + approval_key: "approval-key".to_string(), + approval_grouping_key: "approval-group".to_string(), + persistent_rules: vec![rule.clone()], + }, + ) + .await; + + assert_eq!( + engine.recv_approval_event().await, + Some(crate::core::engine::MockApprovalEvent::Approved { + id: "tool-allow".to_string() + }) + ); + let store = codewhale_config::ConfigStore::load(Some(config_path)).expect("load config store"); + assert_eq!(store.permissions().rules, vec![rule]); + assert!( + app.status_message + .as_deref() + .is_some_and(|message| message.contains("Saved 1 allow permission rule")) + ); + + let exact = config + .exec_policy_engine + .check(codewhale_execpolicy::ExecPolicyContext { + command: "cargo test", + cwd: tmp.path().to_string_lossy().as_ref(), + tool: Some("exec_shell"), + path: None, + ask_for_approval: codewhale_execpolicy::AskForApproval::OnRequest, + sandbox_mode: None, + }) + .expect("check persisted allow"); + assert_eq!( + exact.matched_action, + Some(codewhale_execpolicy::PermissionAction::Allow) + ); + assert!(!exact.requires_approval); + + let expanded = config + .exec_policy_engine + .check(codewhale_execpolicy::ExecPolicyContext { + command: "cargo test --workspace", + cwd: tmp.path().to_string_lossy().as_ref(), + tool: Some("exec_shell"), + path: None, + ask_for_approval: codewhale_execpolicy::AskForApproval::OnRequest, + sandbox_mode: None, + }) + .expect("check expanded command"); + assert!(expanded.requires_approval); +} + #[test] fn second_thinking_block_appends_new_entry_in_same_active_cell() { // Real V4 turns can emit Thinking → Tool → Thinking → Tool before any diff --git a/crates/tui/src/tui/views/mod.rs b/crates/tui/src/tui/views/mod.rs index bdb4a7ff5e..2bd47e9885 100644 --- a/crates/tui/src/tui/views/mod.rs +++ b/crates/tui/src/tui/views/mod.rs @@ -640,8 +640,8 @@ pub enum ViewEvent { approval_key: String, /// Lossy / arity-aware fingerprint, used to scope *approvals*. approval_grouping_key: String, - /// Ask-only permission rules to append when the decision approves. - persistent_ask_rules: Vec, + /// Permission rules to append when the decision approves. + persistent_rules: Vec, }, ElevationDecision { tool_id: String, diff --git a/crates/tui/src/tui/widgets/mod.rs b/crates/tui/src/tui/widgets/mod.rs index 6917123a22..3964921da6 100644 --- a/crates/tui/src/tui/widgets/mod.rs +++ b/crates/tui/src/tui/widgets/mod.rs @@ -1916,10 +1916,23 @@ impl<'a> ApprovalWidget<'a> { ])); } - // Preview of the persistent ask-rule the `[s]` shortcut would save - // (#3766). Informational, so it lives in the (scrollable) body. + // Preview the validated persistent-rule candidates. Informational, so + // they live in the scrollable body rather than the action rows. if let Some(preview) = self.request.ask_rule_save_preview() { - push_ask_rule_save_preview(&mut body, &preview, palette_colors.shortcut, area.width); + push_permission_rule_save_preview( + &mut body, + &preview, + palette_colors.shortcut, + area.width, + ); + } + if let Some(preview) = self.request.allow_rule_save_preview() { + push_permission_rule_save_preview( + &mut body, + &preview, + palette_colors.shortcut, + area.width, + ); } let controls = build_approval_controls( @@ -2141,9 +2154,10 @@ fn inline_region_for(area: Rect, body: &[Line<'static>], controls: &[Line<'stati // Never shrink below the rule + controls. At normal terminal heights, // reserve four body rows: header, detail label, at least one command or // preview row, and the truncation hint. Half a viewport is the preferred - // cap; up to three quarters is allowed only when necessary to retain that - // load-bearing preview on a short frame. Truly tiny frames prioritize the - // complete action set and details chord. + // cap; up to four fifths is allowed only when necessary to retain that + // load-bearing preview on a short frame. The extra permanent-grant row + // needs one more reserved line than the legacy four-action card. Truly + // tiny frames prioritize the complete action set and details chord. let controls_floor = 1u16.saturating_add(control_rows).min(area.height); let preview_rows = if area.height >= 16 { body_rows.min(4) @@ -2152,7 +2166,7 @@ fn inline_region_for(area: Rect, body: &[Line<'static>], controls: &[Line<'stati }; let preview_floor = controls_floor.saturating_add(preview_rows).min(area.height); let preferred_cap = area.height.div_ceil(2); - let short_frame_cap = area.height.saturating_mul(3).div_ceil(4); + let short_frame_cap = area.height.saturating_mul(4).div_ceil(5); let max_height = preferred_cap .max(preview_floor.min(short_frame_cap)) .max(controls_floor) @@ -2394,9 +2408,9 @@ fn push_params_detail_line( ])); } -fn push_ask_rule_save_preview( +fn push_permission_rule_save_preview( lines: &mut Vec>, - preview: &crate::tui::approval::AskRuleSavePreview, + preview: &crate::tui::approval::PermissionRuleSavePreview, shortcut: Color, card_width: u16, ) { @@ -2644,11 +2658,8 @@ fn footer_controls(locale: Locale) -> Cow<'static, str> { Cow::Owned(tr(locale, MessageId::ApprovalControlsHint).replace("{details}", details.as_ref())) } -fn save_ask_rule_hint(locale: Locale) -> &'static str { - match locale { - Locale::ZhHans => " s 批准并保存询问规则", - _ => " s approve + save ask rule", - } +fn save_ask_rule_hint(locale: Locale) -> Cow<'static, str> { + tr(locale, MessageId::ApprovalSaveAskRuleHint) } #[derive(Clone)] @@ -2714,7 +2725,18 @@ fn approval_options_for_request( if request.tool_name == "workflow" { workflow_approval_options(risk, locale).to_vec() } else { - approval_options_for(risk, locale).to_vec() + let mut options = approval_options_for(risk, locale).to_vec(); + if request.can_save_allow_rule() { + options.insert( + 2, + ApprovalOptionRow { + label: tr(locale, MessageId::ApprovalOptionAllowExactRepo), + key_hint: "p", + dangerous: false, + }, + ); + } + options } } @@ -7118,8 +7140,8 @@ mod tests { fn approval_option_two_reads_as_session_scoped_not_always() { // #3766: option 2 / `a` maps to ReviewDecision::ApprovedForSession, so // neither the full option rows nor the compact controls may tell the - // user the approval is "always"/permanent. Persisting is the separate - // `s` save-rule action. + // user that particular option is "always"/permanent. The distinct + // `[p]` row may use that word for an exact repo-scoped grant. let request = crate::tui::approval::ApprovalRequest::new( "approval-1", "exec_shell", @@ -7130,26 +7152,28 @@ mod tests { // Full card (tall): full option rows render the session-scoped label. let full = render_approval_request(&request, Rect::new(0, 0, 100, 30)); + let full_session_option = full + .lines() + .find(|line| line.contains("[2 / a]")) + .expect("full approval card should render the session option"); assert!( - full.to_lowercase().contains("this session"), - "full approval option must state session scope:\n{full}" - ); - assert!( - !full.to_lowercase().contains("always"), - "full approval card must not call the session option 'always':\n{full}" + full_session_option.to_lowercase().contains("this session") + && !full_session_option.to_lowercase().contains("always"), + "full approval option must state session scope without saying always:\n{full}" ); // Short terminal: the reserved controls still render the session-scoped - // option `[2 / a]` and never call it "always". + // option `[2 / a]` without calling that option "always". let compact = render_approval_request(&request, Rect::new(0, 0, 60, 17)); + let compact_session_option = compact + .lines() + .find(|line| line.contains("[2 / a]")) + .expect("short approval card should render the session option"); assert!( - compact.contains("[2 / a]") && compact.to_lowercase().contains("session"), + compact_session_option.to_lowercase().contains("session") + && !compact_session_option.to_lowercase().contains("always"), "short-terminal controls must label [2 / a] as session-scoped:\n{compact}" ); - assert!( - !compact.to_lowercase().contains("always"), - "short-terminal controls must not label the session option 'always':\n{compact}" - ); } #[test] @@ -7195,13 +7219,23 @@ mod tests { let rendered = render_approval_request(&request, Rect::new(0, 0, 120, 40)); - assert!(rendered.contains("s approve + save ask rule"), "{rendered}"); + assert!( + rendered.contains("s allow once + always ask exact rule"), + "{rendered}" + ); + assert!( + rendered.contains("Always allow this exact rule in this repo"), + "{rendered}" + ); assert!(rendered.contains("Save:"), "{rendered}"); assert!(rendered.contains("1 ask rule"), "{rendered}"); + assert!(rendered.contains("1 allow rule"), "{rendered}"); assert!( rendered.contains("tool=exec_shell command=cargo test --workspace"), "{rendered}" ); + assert!(rendered.contains("command_exact=true"), "{rendered}"); + assert!(rendered.contains("workspace=/workspace"), "{rendered}"); } #[test] @@ -7239,6 +7273,10 @@ mod tests { assert!(rendered.contains("Save:"), "{tool_name}:\n{rendered}"); assert!(rendered.contains("1 ask rule"), "{tool_name}:\n{rendered}"); + assert!( + rendered.contains("1 allow rule"), + "{tool_name}:\n{rendered}" + ); assert!( rendered.contains(expected_rule), "{tool_name} should preview {expected_rule}:\n{rendered}" @@ -7272,6 +7310,7 @@ diff --git a/src/b.rs b/src/b.rs\n\ assert!(rendered.contains("Save:"), "{rendered}"); assert!(rendered.contains("2 ask rules"), "{rendered}"); + assert!(rendered.contains("2 allow rules"), "{rendered}"); assert!( rendered.contains("tool=apply_patch path=src/a.rs"), "{rendered}" @@ -7338,7 +7377,7 @@ diff --git a/src/b.rs b/src/b.rs\n\ let rendered = render_approval_request(&request, Rect::new(0, 0, 120, 40)); assert!( - !rendered.contains("s approve + save ask rule"), + !rendered.contains("s allow once + always ask exact rule"), "S shortcut should stay hidden:\n{rendered}" ); assert!( diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index d6dc972795..35f60bff8a 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -1316,8 +1316,12 @@ If you are upgrading from older releases: - `permissions.toml` (sibling file, optional): typed permission rule records loaded next to `config.toml`, for example `~/.codewhale/permissions.toml`. Manually authored `[[rules]]` entries accept `tool`, optional `command` or - `path`, and optional `action = "deny" | "ask" | "allow"`; omitted `action` - defaults to `"ask"`. `deny` blocks matching invocations before mode-based + `path`, optional absolute `workspace`, optional `command_exact = true`, and + optional `action = "deny" | "ask" | "allow"`; omitted `action` defaults to + `"ask"`. `workspace` limits a rule to that repository, while + `command_exact = true` changes a command rule from the historical + arity-aware prefix match to a complete-command match. `deny` blocks matching + invocations before mode-based approval handling, `allow` skips approval for matching invocations, and `ask` forces approval only in modes that can prompt. Outside the TUI auto-approve path, a matching `ask` rule under `approval_policy = "never"` @@ -1325,20 +1329,28 @@ If you are upgrading from older releases: `ask` rules do not downgrade the session into prompting or blocking; explicit `deny` rules still block according to the current execution-policy logic. - In a supported approval card, press `S` to approve the request once and - append exact `action = "ask"` rules to this file. Supported saves are - intentionally narrow: + In a supported approval card, press `S` to allow the request once and append + exact `action = "ask"` rules to this file. For eligible safe requests, choose + **Always allow this exact rule in this repo** (shortcut `P`) to append an + `action = "allow"` rule with the current absolute `workspace` scope. + Remembered shell grants set `command_exact = true`, so later commands with + extra arguments do not inherit the grant. File and patch grants retain the + exact workspace-relative paths produced by the existing validation path. + Supported saves are intentionally narrow: `exec_shell` stores the exact approved command string; `write_file` and `edit_file` store the exact workspace-relative file path; `apply_patch` stores one exact workspace-relative `path` rule per validated touched file from apply-patch preflight. Existing exec command matching remains - arity-aware, and file paths are normalized to the same workspace-relative - form used by runtime matching. + arity-aware for manually authored prefix rules; approval-card allow grants + use complete-command matching. File paths are normalized to the same + workspace-relative form used by runtime matching. `read_file` rules can still be authored manually when you want future reads of a specific path to ask, allow, or deny, but the approval UI does not save - `read_file` rules. The UI is not a policy editor: it does not save - `allow`/`deny`, edit or delete rules, expand globs, or create broad + `read_file` rules. Commands classified as requiring approval or dangerous, + critical approval cards, and repo-law prompts cannot save allow grants and + continue to require review. The UI is not a policy editor: it does not save + deny rules, edit or delete rules, expand globs, or create broad directory/recursive rules. - `[[hotbar]]` (array of tables, optional): user-owned 1-8 slot bindings for the TUI hotbar. Each entry has `slot`, `action`, and optional `label`. From e2864f8ee3fdc194b77359b135e2f84d27bd9639 Mon Sep 17 00:00:00 2001 From: greyfreedom Date: Fri, 24 Jul 2026 18:58:23 +0800 Subject: [PATCH 2/3] fix(tui): repair cross-platform approval grant CI Update real-PTY approval expectations for the renamed allow-once copy and the additional exact-repo option. Gate the process-table OnceLock import to Unix so Windows warning-as-error builds remain clean. --- crates/tui/src/fleet/host.rs | 3 ++- crates/tui/tests/qa_pty.rs | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/tui/src/fleet/host.rs b/crates/tui/src/fleet/host.rs index 51def76d14..2949a521d0 100644 --- a/crates/tui/src/fleet/host.rs +++ b/crates/tui/src/fleet/host.rs @@ -11,7 +11,6 @@ use std::fs::{File, OpenOptions}; use std::io::{Read, Seek, SeekFrom}; use std::path::{Path, PathBuf}; use std::process::{Child, Command, ExitStatus, Stdio}; -use std::sync::OnceLock; use std::thread; use std::time::{Duration, Instant}; @@ -22,6 +21,8 @@ use thiserror::Error; use std::os::unix::process::CommandExt; #[cfg(windows)] use std::os::windows::io::AsRawHandle; +#[cfg(unix)] +use std::sync::OnceLock; #[cfg(windows)] use windows::Win32::Foundation::{CloseHandle, HANDLE}; #[cfg(windows)] diff --git a/crates/tui/tests/qa_pty.rs b/crates/tui/tests/qa_pty.rs index 3a46ed45e6..d3dd6993e9 100644 --- a/crates/tui/tests/qa_pty.rs +++ b/crates/tui/tests/qa_pty.rs @@ -1278,7 +1278,7 @@ fn approval_modal_keeps_wheel_for_review_and_denies_without_side_effect() -> any h.wait_for_text(prompt, KEY_TIMEOUT)?; h.wait_for_idle(Duration::from_millis(100), Duration::from_secs(2))?; h.send(keys::key::enter())?; - h.wait_for_text("Approve once", Duration::from_secs(10))?; + h.wait_for_text("Allow once", Duration::from_secs(10))?; h.wait_for_text("Deny this call", KEY_TIMEOUT)?; let (deny_row, deny_col) = h @@ -1304,6 +1304,7 @@ fn approval_modal_keeps_wheel_for_review_and_denies_without_side_effect() -> any h.wait_for_text("❯ [1 / y]", KEY_TIMEOUT)?; h.send(keys::key::down())?; h.send(keys::key::down())?; + h.send(keys::key::down())?; h.wait_for_text("❯ [3 / d / n]", KEY_TIMEOUT)?; h.send(keys::mouse::click(deny_row, deny_col))?; if let Err(err) = h.wait_for_text("DENIAL-HONORED", Duration::from_secs(10)) { @@ -2471,7 +2472,7 @@ fn work_surface_file_mutation_modes_are_truthful_in_real_pty_frames() -> anyhow: h.wait_for_text(prompt, KEY_TIMEOUT)?; h.send(keys::key::enter())?; if approve_once { - h.wait_for_text("Approve once", Duration::from_secs(10))?; + h.wait_for_text("Allow once", Duration::from_secs(10))?; h.send(b"y")?; } h.wait_for_text("FILE-MUTATION-FIXTURE-DONE", Duration::from_secs(20))?; From 702432914378c3cb8f413064cb6314d7ba670eab Mon Sep 17 00:00:00 2001 From: greyfreedom Date: Fri, 24 Jul 2026 19:30:09 +0800 Subject: [PATCH 3/3] test(config): make allow scope assertion portable Avoid assuming Rust Debug quoting matches TOML output for Windows paths. Keep the rendered workspace-field check while relying on the typed round-trip assertion for the exact persisted value. --- crates/config/src/tests.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/config/src/tests.rs b/crates/config/src/tests.rs index f29c159ffd..acbaca711d 100644 --- a/crates/config/src/tests.rs +++ b/crates/config/src/tests.rs @@ -734,7 +734,10 @@ fn config_store_appends_exact_workspace_allow_rules() { assert!(body.contains("# remembered grants stay user-owned")); assert!(body.contains("action = \"allow\"")); assert!(body.contains("command_exact = true")); - assert!(body.contains(&format!("workspace = {:?}", dir.path().to_string_lossy()))); + // Windows paths may make toml_edit choose a different valid quoting style + // than Rust's Debug output. Check the rendered field shape here and its + // exact value through the typed parse below. + assert!(body.lines().any(|line| line.starts_with("workspace = "))); let parsed: PermissionsToml = toml::from_str(&body).expect("parse permissions"); assert_eq!(parsed.rules, vec![rule.clone()]);