diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4203a17cd..1d74f4ca8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -57,7 +57,7 @@ jobs: - uses: Swatinem/rust-cache@v2 if: runner.os != 'Linux' with: - cache-bin: false + cache-bin: "false" - name: Run tests if: runner.os != 'Linux' run: cargo test --workspace --all-features --locked @@ -90,7 +90,7 @@ jobs: - uses: Swatinem/rust-cache@v2 if: runner.os != 'Linux' with: - cache-bin: false + cache-bin: "false" - name: Build wrapper binaries if: runner.os != 'Linux' run: cargo build --release --locked -p codewhale-cli -p codewhale-tui @@ -120,7 +120,7 @@ jobs: sudo apt-get install -y libdbus-1-dev pkg-config - uses: Swatinem/rust-cache@v2 with: - cache-bin: false + cache-bin: "false" - name: Build docs run: cargo doc --workspace --no-deps env: diff --git a/Cargo.lock b/Cargo.lock index 2d5bd8e14..4a686dd6f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -861,6 +861,7 @@ name = "codewhale-config" version = "0.8.44" dependencies = [ "anyhow", + "codewhale-execpolicy", "codewhale-secrets", "dirs", "serde", @@ -892,6 +893,7 @@ version = "0.8.44" dependencies = [ "anyhow", "codewhale-protocol", + "globset", "serde", ] @@ -978,6 +980,7 @@ dependencies = [ "clap", "clap_complete", "codewhale-config", + "codewhale-execpolicy", "codewhale-secrets", "codewhale-tools", "colored", diff --git a/config.example.toml b/config.example.toml index 87af1a8e4..79fd55be9 100644 --- a/config.example.toml +++ b/config.example.toml @@ -141,6 +141,21 @@ sandbox_mode = "workspace-write" # read-only | workspace-write | danger-full-acc # auto_allow = ["cargo check", "npm run"] # # auto_allow = [] +# auto_deny = ["rm -rf"] +# +# Typed persistent permission rules. These are intentionally conservative: +# shell commands use the same arity-aware matching as auto_allow, and file +# paths are workspace-relative globs. +# +# [[permissions.rules]] +# tool = "exec_shell" +# decision = "allow" # "allow", "deny", or "ask" +# command = "cargo test" +# +# [[permissions.rules]] +# tool = "edit_file" +# decision = "ask" +# path = "src/**" max_subagents = 10 # optional (1-20) # Optional sub-agent tuning. max_concurrent overrides top-level max_subagents. diff --git a/crates/app-server/src/lib.rs b/crates/app-server/src/lib.rs index e580ed322..ff403703a 100644 --- a/crates/app-server/src/lib.rs +++ b/crates/app-server/src/lib.rs @@ -9,7 +9,6 @@ use axum::{Json, Router}; use codewhale_agent::ModelRegistry; use codewhale_config::{CliRuntimeOverrides, ConfigStore}; use codewhale_core::Runtime; -use codewhale_execpolicy::ExecPolicyEngine; use codewhale_hooks::{HookDispatcher, JsonlHookSink, StdoutHookSink}; use codewhale_mcp::McpManager; use codewhale_protocol::{ @@ -285,7 +284,7 @@ fn build_state(config_path: Option) -> Result { state_store, Arc::new(ToolRegistry::default()), Arc::new(McpManager::default()), - ExecPolicyEngine::new(Vec::new(), Vec::new()), + config.exec_policy_engine(), hooks, ); diff --git a/crates/config/Cargo.toml b/crates/config/Cargo.toml index 2d9ea5228..4e27366e6 100644 --- a/crates/config/Cargo.toml +++ b/crates/config/Cargo.toml @@ -8,6 +8,7 @@ description = "Config schema and precedence model for DeepSeek workspace archite [dependencies] anyhow.workspace = true +codewhale-execpolicy = { path = "../execpolicy", version = "0.8.44" } codewhale-secrets = { path = "../secrets", version = "0.8.44" } dirs.workspace = true serde.workspace = true diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index 9bfb089e0..d8b316fdd 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -6,6 +6,8 @@ use std::path::{Component, Path, PathBuf}; use std::sync::OnceLock; use anyhow::{Context, Result, bail}; +use codewhale_execpolicy::{ExecPolicyEngine, Ruleset}; +pub use codewhale_execpolicy::{PermissionDecision, ToolPermissionRule}; use codewhale_secrets::SecretSource; pub use codewhale_secrets::Secrets; use serde::{Deserialize, Serialize}; @@ -182,6 +184,19 @@ impl ProvidersToml { } } +#[derive(Debug, Clone, Serialize, Deserialize, Default, PartialEq, Eq)] +pub struct PermissionsToml { + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub rules: Vec, +} + +impl PermissionsToml { + #[must_use] + pub fn is_empty(&self) -> bool { + self.rules.is_empty() + } +} + #[derive(Debug, Clone, Serialize, Deserialize, Default)] pub struct ConfigToml { /// TUI-compatible DeepSeek API key. Kept at the root so both `deepseek` @@ -205,6 +220,16 @@ pub struct ConfigToml { pub telemetry: Option, pub approval_policy: Option, pub sandbox_mode: Option, + /// Legacy command allow-list. Entries become `exec_shell` allow rules. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub auto_allow: Vec, + /// Legacy command deny-list. Entries become `exec_shell` deny rules. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub auto_deny: Vec, + /// Typed tool permission rules. First version supports tool name, command + /// prefix, and workspace-relative path glob matching. + #[serde(default, skip_serializing_if = "PermissionsToml::is_empty")] + pub permissions: PermissionsToml, #[serde(default)] pub providers: ProvidersToml, /// Per-domain network policy (#135). When absent, network tools fall back @@ -371,6 +396,15 @@ impl ConfigToml { if project.sandbox_mode.is_some() { self.sandbox_mode = project.sandbox_mode; } + if !project.auto_allow.is_empty() { + self.auto_allow = project.auto_allow; + } + if !project.auto_deny.is_empty() { + self.auto_deny = project.auto_deny; + } + if !project.permissions.is_empty() { + self.permissions = project.permissions; + } // Provider is only overridden if explicitly set (non-default). if project.provider != ProviderKind::Deepseek || has_api_key { self.provider = project.provider; @@ -418,6 +452,17 @@ impl ConfigToml { } } + #[must_use] + pub fn permission_ruleset(&self) -> Ruleset { + Ruleset::user(self.auto_allow.clone(), self.auto_deny.clone()) + .with_rules(self.permissions.rules.clone()) + } + + #[must_use] + pub fn exec_policy_engine(&self) -> ExecPolicyEngine { + ExecPolicyEngine::with_rulesets(vec![self.permission_ruleset()]) + } + #[must_use] pub fn get_value(&self, key: &str) -> Option { match key { @@ -435,6 +480,8 @@ impl ConfigToml { "telemetry" => self.telemetry.map(|v| v.to_string()), "approval_policy" => self.approval_policy.clone(), "sandbox_mode" => self.sandbox_mode.clone(), + "auto_allow" => serialize_string_list(&self.auto_allow), + "auto_deny" => serialize_string_list(&self.auto_deny), "providers.deepseek.api_key" => self.providers.deepseek.api_key.clone(), "providers.deepseek.base_url" => self.providers.deepseek.base_url.clone(), "providers.deepseek.model" => self.providers.deepseek.model.clone(), @@ -1621,6 +1668,13 @@ fn serialize_http_headers(headers: &BTreeMap) -> Option ) } +fn serialize_string_list(values: &[String]) -> Option { + if values.is_empty() { + return None; + } + Some(values.join(",")) +} + fn redact_secret(secret: &str) -> String { let chars: Vec = secret.chars().collect(); if chars.len() <= 16 { @@ -1814,6 +1868,77 @@ mod tests { assert!(policy.audit); } + #[test] + fn permissions_toml_deserializes_typed_rules_and_legacy_lists() { + let config: ConfigToml = toml::from_str( + r#" + auto_allow = ["git status"] + auto_deny = ["rm -rf"] + + [[permissions.rules]] + tool = "exec_shell" + decision = "allow" + command = "cargo test" + + [[permissions.rules]] + tool = "edit_file" + decision = "ask" + path = "src/**" + "#, + ) + .expect("permissions toml"); + + assert_eq!(config.auto_allow, ["git status"]); + assert_eq!(config.auto_deny, ["rm -rf"]); + assert_eq!(config.permissions.rules.len(), 2); + assert_eq!( + config.permissions.rules[0], + ToolPermissionRule::exec_shell(PermissionDecision::Allow, "cargo test") + ); + assert_eq!( + config.permissions.rules[1], + ToolPermissionRule::file_path("edit_file", PermissionDecision::Ask, "src/**") + ); + } + + #[test] + fn config_builds_exec_policy_engine_with_legacy_and_typed_rules() { + let config: ConfigToml = toml::from_str( + r#" + auto_allow = ["git status"] + + [[permissions.rules]] + tool = "exec_shell" + decision = "deny" + command = "git status --dangerous" + "#, + ) + .expect("permissions toml"); + let engine = config.exec_policy_engine(); + + let allowed = engine + .check(codewhale_execpolicy::ExecPolicyContext { + command: "git status --porcelain", + cwd: ".", + ask_for_approval: codewhale_execpolicy::AskForApproval::UnlessTrusted, + sandbox_mode: Some("workspace-write"), + }) + .expect("policy decision"); + assert!(allowed.allow); + assert!(!allowed.requires_approval); + + let denied = engine + .check(codewhale_execpolicy::ExecPolicyContext { + command: "git status --dangerous", + cwd: ".", + ask_for_approval: codewhale_execpolicy::AskForApproval::UnlessTrusted, + sandbox_mode: Some("workspace-write"), + }) + .expect("policy decision"); + assert!(!denied.allow); + assert_eq!(denied.requirement.phase(), "forbidden"); + } + struct EnvGuard { deepseek_api_key: Option, deepseek_base_url: Option, diff --git a/crates/execpolicy/Cargo.toml b/crates/execpolicy/Cargo.toml index 669759c49..bf89de939 100644 --- a/crates/execpolicy/Cargo.toml +++ b/crates/execpolicy/Cargo.toml @@ -9,4 +9,5 @@ description = "Execution policy and approval model parity for DeepSeek workspace [dependencies] anyhow.workspace = true codewhale-protocol = { path = "../protocol", version = "0.8.44" } +globset = "0.4.18" serde.workspace = true diff --git a/crates/execpolicy/src/lib.rs b/crates/execpolicy/src/lib.rs index 14466124a..c60a9e5b9 100644 --- a/crates/execpolicy/src/lib.rs +++ b/crates/execpolicy/src/lib.rs @@ -5,10 +5,12 @@ use std::collections::HashSet; use anyhow::Result; use bash_arity::BashArityDict; use codewhale_protocol::{NetworkPolicyAmendment, NetworkPolicyRuleAction}; +use globset::GlobBuilder; use serde::{Deserialize, Serialize}; /// Priority layer for a permission ruleset. Higher ordinal = higher priority. -/// On conflict, the highest-priority layer's longest matching prefix wins. +/// Deny rules still win across layers; for non-deny ties the higher-priority +/// layer wins before pattern specificity is considered. #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum RulesetLayer { @@ -17,12 +19,100 @@ pub enum RulesetLayer { User = 2, } +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum PermissionDecision { + Allow, + Deny, + Ask, +} + +impl PermissionDecision { + fn precedence(self) -> u8 { + match self { + Self::Deny => 3, + Self::Ask => 2, + Self::Allow => 1, + } + } + + fn as_str(self) -> &'static str { + match self { + Self::Allow => "allow", + Self::Deny => "deny", + Self::Ask => "ask", + } + } +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct ToolPermissionRule { + pub tool: String, + pub decision: PermissionDecision, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub command: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub path: Option, +} + +impl ToolPermissionRule { + #[must_use] + pub fn new(tool: impl Into, decision: PermissionDecision) -> Self { + Self { + tool: tool.into(), + decision, + command: None, + path: None, + } + } + + #[must_use] + pub fn exec_shell(decision: PermissionDecision, command: impl Into) -> Self { + Self { + tool: "exec_shell".to_string(), + decision, + command: Some(command.into()), + path: None, + } + } + + #[must_use] + pub fn file_path( + tool: impl Into, + decision: PermissionDecision, + path: impl Into, + ) -> Self { + Self { + tool: tool.into(), + decision, + command: None, + path: Some(path.into()), + } + } + + #[must_use] + pub fn pattern_label(&self) -> String { + let mut parts = vec![format!("tool '{}'", self.tool)]; + if let Some(command) = self.command.as_deref() { + parts.push(format!("command '{command}'")); + } + if let Some(path) = self.path.as_deref() { + parts.push(format!("path '{path}'")); + } + parts.join(", ") + } +} + /// A named set of allow/deny prefix rules at a given priority layer. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct Ruleset { pub layer: RulesetLayer, + #[serde(default)] pub trusted_prefixes: Vec, + #[serde(default)] pub denied_prefixes: Vec, + #[serde(default)] + pub rules: Vec, } impl Ruleset { @@ -31,6 +121,7 @@ impl Ruleset { layer: RulesetLayer::BuiltinDefault, trusted_prefixes: vec![], denied_prefixes: vec![], + rules: vec![], } } @@ -39,6 +130,7 @@ impl Ruleset { layer: RulesetLayer::Agent, trusted_prefixes: trusted, denied_prefixes: denied, + rules: vec![], } } @@ -47,8 +139,15 @@ impl Ruleset { layer: RulesetLayer::User, trusted_prefixes: trusted, denied_prefixes: denied, + rules: vec![], } } + + #[must_use] + pub fn with_rules(mut self, rules: Vec) -> Self { + self.rules = rules; + self + } } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] @@ -126,6 +225,52 @@ pub struct ExecPolicyContext<'a> { pub sandbox_mode: Option<&'a str>, } +#[derive(Debug, Clone)] +pub struct ToolPermissionContext<'a> { + pub tool: &'a str, + pub command: Option<&'a str>, + pub path: Option<&'a str>, +} + +impl<'a> ToolPermissionContext<'a> { + #[must_use] + pub fn exec_shell(command: &'a str) -> Self { + Self { + tool: "exec_shell", + command: Some(command), + path: None, + } + } +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct MatchedToolPermissionRule { + pub layer: RulesetLayer, + pub rule: ToolPermissionRule, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct ToolPermissionCheck { + pub decision: Option, + pub matched_rule: Option, +} + +impl ToolPermissionCheck { + #[must_use] + pub fn unmatched() -> Self { + Self { + decision: None, + matched_rule: None, + } + } +} + +#[derive(Debug, Clone)] +struct LayeredToolPermissionRule { + layer: RulesetLayer, + rule: ToolPermissionRule, +} + #[derive(Debug, Clone, Default)] pub struct ExecPolicyEngine { /// Layered rulesets (builtin → agent → user). When non-empty, takes precedence @@ -170,28 +315,40 @@ impl ExecPolicyEngine { self.rulesets.sort_by_key(|r| r.layer); } - /// Resolve the effective trusted/denied prefix sets by merging all rulesets. - /// - /// Collects all prefixes from every layer (builtin → agent → user) into flat - /// trusted/denied lists. The `check()` method then applies deny-always-wins - /// semantics: any matching deny prefix blocks the command regardless of layer. - /// Trusted rules are only consulted after deny checks pass. - fn resolve_prefixes(&self) -> (Vec, Vec) { + fn layered_permission_rules(&self) -> Vec { + let mut rules = Vec::new(); if self.rulesets.is_empty() { - return (self.trusted_prefixes.clone(), self.denied_prefixes.clone()); + rules.extend(legacy_command_rules( + RulesetLayer::User, + &self.trusted_prefixes, + &self.denied_prefixes, + )); + return rules; } - // Collect all trusted/denied across all layers, highest-priority last so they - // shadow lower-priority entries with the same prefix. - let mut trusted: Vec = vec![]; - let mut denied: Vec = vec![]; - for rs in &self.rulesets { - trusted.extend(rs.trusted_prefixes.iter().cloned()); - denied.extend(rs.denied_prefixes.iter().cloned()); + + for ruleset in &self.rulesets { + rules.extend( + ruleset + .rules + .iter() + .cloned() + .map(|rule| LayeredToolPermissionRule { + layer: ruleset.layer, + rule, + }), + ); + rules.extend(legacy_command_rules( + ruleset.layer, + &ruleset.trusted_prefixes, + &ruleset.denied_prefixes, + )); } - // Also merge legacy flat lists as user-layer. - trusted.extend(self.trusted_prefixes.iter().cloned()); - denied.extend(self.denied_prefixes.iter().cloned()); - (trusted, denied) + rules.extend(legacy_command_rules( + RulesetLayer::User, + &self.trusted_prefixes, + &self.denied_prefixes, + )); + rules } pub fn remember_session_approval(&mut self, approval_key: String) { @@ -202,62 +359,104 @@ impl ExecPolicyEngine { self.approved_for_session.contains(approval_key) } + #[must_use] + pub fn check_tool_permission(&self, ctx: ToolPermissionContext<'_>) -> ToolPermissionCheck { + let mut best: Option<(LayeredToolPermissionRule, usize)> = None; + + for candidate in self.layered_permission_rules() { + if !tool_rule_matches(&candidate.rule, &ctx, &self.arity_dict) { + continue; + } + let specificity = rule_specificity(&candidate.rule); + let should_replace = best.as_ref().is_none_or(|(current, current_specificity)| { + compare_rule_priority(&candidate, specificity, current, *current_specificity) + }); + if should_replace { + best = Some((candidate, specificity)); + } + } + + match best { + Some((matched, _)) => ToolPermissionCheck { + decision: Some(matched.rule.decision), + matched_rule: Some(MatchedToolPermissionRule { + layer: matched.layer, + rule: matched.rule, + }), + }, + None => ToolPermissionCheck::unmatched(), + } + } + pub fn check(&self, ctx: ExecPolicyContext<'_>) -> Result { - let normalized = normalize_command(ctx.command); - let (trusted_prefixes, denied_prefixes) = self.resolve_prefixes(); - // Deny rules use simple prefix matching (no arity semantics needed). - if let Some(rule) = denied_prefixes - .iter() - .find(|rule| normalized.starts_with(&normalize_command(rule))) + let tool_rule = self.check_tool_permission(ToolPermissionContext::exec_shell(ctx.command)); + if let Some(matched) = tool_rule + .matched_rule + .as_ref() + .filter(|matched| matched.rule.decision == PermissionDecision::Deny) { return Ok(ExecPolicyDecision { allow: false, requires_approval: false, - matched_rule: Some(rule.clone()), + matched_rule: Some(matched.rule.pattern_label()), requirement: ExecApprovalRequirement::Forbidden { - reason: format!("Command blocked by denied prefix rule '{rule}'"), + reason: format!( + "Command blocked by {} rule ({})", + matched.rule.decision.as_str(), + matched.rule.pattern_label() + ), }, }); } - // Allow (trusted) rules use arity-aware prefix matching so that - // `auto_allow = ["git status"]` matches `git status -s` but NOT - // `git push origin main`. - let trusted_rule = trusted_prefixes - .iter() - .find(|rule| self.arity_dict.allow_rule_matches(rule, ctx.command)) - .cloned(); - let is_trusted = trusted_rule.is_some(); - let requirement = match ctx.ask_for_approval { AskForApproval::Never => ExecApprovalRequirement::Skip { bypass_sandbox: false, proposed_execpolicy_amendment: None, }, - AskForApproval::UnlessTrusted if is_trusted => ExecApprovalRequirement::Skip { - bypass_sandbox: false, - proposed_execpolicy_amendment: None, + AskForApproval::Reject { rules, .. } if rules => ExecApprovalRequirement::Forbidden { + reason: "Policy is configured to reject rule-exceptions.".to_string(), }, + _ if matches!(tool_rule.decision, Some(PermissionDecision::Ask)) => { + ExecApprovalRequirement::NeedsApproval { + reason: tool_rule + .matched_rule + .as_ref() + .map(|matched| { + format!( + "Approval required by ask rule ({})", + matched.rule.pattern_label() + ) + }) + .unwrap_or_else(|| "Approval required by ask rule.".to_string()), + proposed_execpolicy_amendment: None, + proposed_network_policy_amendments: vec![], + } + } + AskForApproval::OnRequest + if matches!(tool_rule.decision, Some(PermissionDecision::Allow)) => + { + ExecApprovalRequirement::NeedsApproval { + reason: "Approval requested by policy mode.".to_string(), + proposed_execpolicy_amendment: None, + proposed_network_policy_amendments: vec![], + } + } + _ if matches!(tool_rule.decision, Some(PermissionDecision::Allow)) => { + ExecApprovalRequirement::Skip { + bypass_sandbox: false, + proposed_execpolicy_amendment: None, + } + } AskForApproval::OnFailure => ExecApprovalRequirement::Skip { bypass_sandbox: false, proposed_execpolicy_amendment: None, }, - AskForApproval::Reject { rules, .. } if rules => ExecApprovalRequirement::Forbidden { - reason: "Policy is configured to reject rule-exceptions.".to_string(), - }, _ => ExecApprovalRequirement::NeedsApproval { - reason: if is_trusted { - "Approval requested by policy mode.".to_string() - } else { - "Unmatched command prefix requires approval.".to_string() - }, - proposed_execpolicy_amendment: if is_trusted { - None - } else { - Some(ExecPolicyAmendment { - prefixes: vec![first_token(ctx.command)], - }) - }, + reason: "Unmatched command prefix requires approval.".to_string(), + proposed_execpolicy_amendment: Some(ExecPolicyAmendment { + prefixes: vec![first_token(ctx.command)], + }), proposed_network_policy_amendments: vec![NetworkPolicyAmendment { host: ctx.cwd.to_string(), action: NetworkPolicyRuleAction::Allow, @@ -274,14 +473,177 @@ impl ExecPolicyEngine { Ok(ExecPolicyDecision { allow, requires_approval, - matched_rule: trusted_rule, + matched_rule: tool_rule + .matched_rule + .as_ref() + .map(|matched| matched.rule.pattern_label()), requirement, }) } } +fn legacy_command_rules( + layer: RulesetLayer, + trusted_prefixes: &[String], + denied_prefixes: &[String], +) -> Vec { + trusted_prefixes + .iter() + .map(|command| LayeredToolPermissionRule { + layer, + rule: ToolPermissionRule::exec_shell(PermissionDecision::Allow, command.clone()), + }) + .chain( + denied_prefixes + .iter() + .map(|command| LayeredToolPermissionRule { + layer, + rule: ToolPermissionRule::exec_shell(PermissionDecision::Deny, command.clone()), + }), + ) + .collect() +} + +fn compare_rule_priority( + candidate: &LayeredToolPermissionRule, + candidate_specificity: usize, + current: &LayeredToolPermissionRule, + current_specificity: usize, +) -> bool { + ( + candidate.rule.decision.precedence(), + candidate.layer, + candidate_specificity, + ) > ( + current.rule.decision.precedence(), + current.layer, + current_specificity, + ) +} + +fn tool_rule_matches( + rule: &ToolPermissionRule, + ctx: &ToolPermissionContext<'_>, + arity_dict: &BashArityDict, +) -> bool { + if !rule.tool.eq_ignore_ascii_case(ctx.tool) { + return false; + } + if let Some(command_rule) = rule.command.as_deref() { + let Some(command) = ctx.command else { + return false; + }; + if !command_rule_matches(rule.decision, command_rule, command, arity_dict) { + return false; + } + } + if let Some(path_rule) = rule.path.as_deref() { + let Some(path) = ctx.path else { + return false; + }; + if !path_pattern_matches(path_rule, path) { + return false; + } + } + true +} + +fn command_rule_matches( + decision: PermissionDecision, + pattern: &str, + command: &str, + arity_dict: &BashArityDict, +) -> bool { + match decision { + PermissionDecision::Deny => command_prefix_matches(pattern, command), + PermissionDecision::Allow | PermissionDecision::Ask => { + arity_dict.allow_rule_matches(pattern, command) + } + } +} + +fn path_pattern_matches(pattern: &str, path: &str) -> bool { + let pattern = normalize_path_pattern(pattern); + let path = normalize_path_pattern(path); + if let Some(prefix) = pattern.strip_suffix("/**") { + if path == prefix || path.starts_with(&format!("{prefix}/")) { + return true; + } + } + if !pattern.contains('*') && !pattern.contains('?') { + return pattern == path; + } + GlobBuilder::new(&pattern) + .literal_separator(true) + .build() + .map(|glob| glob.compile_matcher().is_match(path)) + .unwrap_or(false) +} + +fn rule_specificity(rule: &ToolPermissionRule) -> usize { + let mut score = rule.tool.len(); + if let Some(command) = rule.command.as_deref() { + score += 1_000 + non_wildcard_len(command); + } + if let Some(path) = rule.path.as_deref() { + score += 1_000 + non_wildcard_len(path); + } + score +} + +fn non_wildcard_len(value: &str) -> usize { + value.chars().filter(|ch| *ch != '*' && *ch != '?').count() +} + fn normalize_command(value: &str) -> String { - value.trim().to_ascii_lowercase() + value + .trim() + .to_ascii_lowercase() + .split_whitespace() + .collect::>() + .join(" ") +} + +fn command_prefix_matches(pattern: &str, command: &str) -> bool { + let pattern = normalize_command(pattern); + if pattern.is_empty() { + return false; + } + let command = normalize_command(command); + command + .strip_prefix(&pattern) + .is_some_and(|suffix| suffix.is_empty() || suffix.starts_with(' ')) +} + +fn normalize_path_pattern(value: &str) -> String { + let raw = value.trim().replace('\\', "/"); + let absolute = raw.starts_with('/'); + let mut segments = Vec::new(); + + for segment in raw.split('/') { + match segment { + "" | "." => {} + ".." => { + if absolute { + segments.pop(); + } else if segments.is_empty() || segments.last() == Some(&"..") { + segments.push(segment); + } else { + segments.pop(); + } + } + _ => segments.push(segment), + } + } + + let normalized = segments.join("/"); + if absolute && normalized.is_empty() { + "/".to_string() + } else if absolute { + format!("/{normalized}") + } else { + normalized + } } fn first_token(command: &str) -> String { @@ -305,6 +667,10 @@ mod tests { } } + fn exec_ctx(command: &str) -> ExecPolicyContext<'_> { + ctx(command, AskForApproval::UnlessTrusted) + } + #[test] fn trusted_prefix_skips_approval_when_policy_is_unless_trusted() { let engine = ExecPolicyEngine::new(vec!["git status".to_string()], vec![]); @@ -315,7 +681,10 @@ mod tests { assert!(decision.allow); assert!(!decision.requires_approval); - assert_eq!(decision.matched_rule.as_deref(), Some("git status")); + assert_eq!( + decision.matched_rule.as_deref(), + Some("tool 'exec_shell', command 'git status'") + ); assert!(matches!( decision.requirement, ExecApprovalRequirement::Skip { @@ -338,14 +707,45 @@ mod tests { assert!(!decision.allow); assert!(!decision.requires_approval); - assert_eq!(decision.matched_rule.as_deref(), Some("git status")); + assert_eq!( + decision.matched_rule.as_deref(), + Some("tool 'exec_shell', command 'git status'") + ); assert!(matches!( decision.requirement, ExecApprovalRequirement::Forbidden { .. } )); assert_eq!( decision.reason(), - "Command blocked by denied prefix rule 'git status'" + "Command blocked by deny rule (tool 'exec_shell', command 'git status')" + ); + } + + #[test] + fn denied_prefix_respects_command_word_boundaries() { + let engine = ExecPolicyEngine::new(vec![], vec!["ls".to_string()]); + + let blocked = engine.check(exec_ctx("ls -la")).unwrap(); + assert!(!blocked.allow); + assert_eq!(blocked.requirement.phase(), "forbidden"); + + let separate_binary = engine.check(exec_ctx("ls-remote origin")).unwrap(); + assert!(separate_binary.allow); + assert!(separate_binary.requires_approval); + assert_eq!(separate_binary.matched_rule, None); + } + + #[test] + fn legacy_auto_allow_prefixes_become_exec_shell_rules() { + let engine = ExecPolicyEngine::new(vec!["git status".to_string()], vec![]); + + let decision = engine.check(exec_ctx("git status --porcelain")).unwrap(); + + assert!(decision.allow); + assert!(!decision.requires_approval); + assert_eq!( + decision.matched_rule.as_deref(), + Some("tool 'exec_shell', command 'git status'") ); } @@ -389,7 +789,10 @@ mod tests { assert!(decision.allow); assert!(decision.requires_approval); - assert_eq!(decision.matched_rule.as_deref(), Some("cargo test")); + assert_eq!( + decision.matched_rule.as_deref(), + Some("tool 'exec_shell', command 'cargo test'") + ); match decision.requirement { ExecApprovalRequirement::NeedsApproval { proposed_execpolicy_amendment, @@ -423,4 +826,124 @@ mod tests { "Policy is configured to reject rule-exceptions." ); } + + #[test] + fn legacy_auto_allow_uses_bash_arity() { + let engine = ExecPolicyEngine::new(vec!["git status".to_string()], vec![]); + + let decision = engine.check(exec_ctx("git push origin main")).unwrap(); + + assert!(decision.requires_approval); + assert_eq!(decision.matched_rule, None); + } + + #[test] + fn deny_rule_wins_over_user_allow_rule() { + let engine = ExecPolicyEngine::with_rulesets(vec![ + Ruleset::builtin_default().with_rules(vec![ToolPermissionRule::exec_shell( + PermissionDecision::Deny, + "cargo", + )]), + Ruleset::user(vec![], vec![]).with_rules(vec![ToolPermissionRule::exec_shell( + PermissionDecision::Allow, + "cargo test", + )]), + ]); + + let decision = engine.check(exec_ctx("cargo test --workspace")).unwrap(); + + assert!(!decision.allow); + assert_eq!(decision.requirement.phase(), "forbidden"); + assert_eq!( + decision.matched_rule.as_deref(), + Some("tool 'exec_shell', command 'cargo'") + ); + } + + #[test] + fn ask_rule_wins_over_allow_rule() { + let engine = + ExecPolicyEngine::with_rulesets(vec![Ruleset::user(vec![], vec![]).with_rules(vec![ + ToolPermissionRule::exec_shell(PermissionDecision::Allow, "cargo"), + ToolPermissionRule::exec_shell(PermissionDecision::Ask, "cargo test"), + ])]); + + let decision = engine.check(exec_ctx("cargo test --workspace")).unwrap(); + + assert!(decision.allow); + assert!(decision.requires_approval); + assert_eq!(decision.requirement.phase(), "needs_approval"); + assert_eq!( + decision.matched_rule.as_deref(), + Some("tool 'exec_shell', command 'cargo test'") + ); + } + + #[test] + fn ask_rule_overrides_on_failure_policy() { + let engine = + ExecPolicyEngine::with_rulesets(vec![Ruleset::user(vec![], vec![]).with_rules(vec![ + ToolPermissionRule::exec_shell(PermissionDecision::Ask, "cargo test"), + ])]); + + let decision = engine + .check(ExecPolicyContext { + command: "cargo test --workspace", + cwd: ".", + ask_for_approval: AskForApproval::OnFailure, + sandbox_mode: Some("workspace-write"), + }) + .unwrap(); + + assert!(decision.allow); + assert!(decision.requires_approval); + assert_eq!(decision.requirement.phase(), "needs_approval"); + } + + #[test] + fn path_rules_match_workspace_relative_globs() { + let engine = + ExecPolicyEngine::with_rulesets(vec![Ruleset::user(vec![], vec![]).with_rules(vec![ + ToolPermissionRule::file_path("file_edit", PermissionDecision::Allow, "docs/**"), + ])]); + + let decision = engine.check_tool_permission(ToolPermissionContext { + tool: "file_edit", + command: None, + path: Some("./docs/guide/setup.md"), + }); + + assert_eq!(decision.decision, Some(PermissionDecision::Allow)); + } + + #[test] + fn path_star_does_not_cross_directory_separator() { + assert!(path_pattern_matches("docs/*.md", "docs/readme.md")); + assert!(!path_pattern_matches("docs/*.md", "docs/guides/readme.md")); + } + + #[test] + fn path_rules_normalize_dot_segments() { + assert!(path_pattern_matches("src/**", "src/./lib.rs")); + assert!(!path_pattern_matches("src/**", "src/../secret.txt")); + assert!(!path_pattern_matches("src/**", "../src/lib.rs")); + assert!(path_pattern_matches("/foo", "/../foo")); + assert!(!path_pattern_matches("/src/**", "/src/../secret.txt")); + } + + #[test] + fn tool_name_only_rule_matches_unknown_tool() { + let engine = + ExecPolicyEngine::with_rulesets(vec![Ruleset::user(vec![], vec![]).with_rules(vec![ + ToolPermissionRule::new("agent_spawn", PermissionDecision::Ask), + ])]); + + let decision = engine.check_tool_permission(ToolPermissionContext { + tool: "agent_spawn", + command: None, + path: None, + }); + + assert_eq!(decision.decision, Some(PermissionDecision::Ask)); + } } diff --git a/crates/tui/Cargo.toml b/crates/tui/Cargo.toml index f78b57a42..2b8baf1ea 100644 --- a/crates/tui/Cargo.toml +++ b/crates/tui/Cargo.toml @@ -28,6 +28,7 @@ path = "src/bin/deepseek_tui_legacy_shim.rs" anyhow = "1.0.100" arboard = "3.4" codewhale-config = { path = "../config", version = "0.8.44" } +codewhale-execpolicy = { path = "../execpolicy", version = "0.8.44" } codewhale-secrets = { path = "../secrets", version = "0.8.44" } codewhale-tools = { path = "../tools", version = "0.8.44" } schemaui = { version = "0.12.0", default-features = false, optional = true } diff --git a/crates/tui/src/composer_history.rs b/crates/tui/src/composer_history.rs index 4f8bb1ce9..ff487c9e0 100644 --- a/crates/tui/src/composer_history.rs +++ b/crates/tui/src/composer_history.rs @@ -33,6 +33,9 @@ use std::time::Duration; pub const MAX_HISTORY_ENTRIES: usize = 1000; const HISTORY_FILE_NAME: &str = "composer_history.txt"; +// Prevent a steady stream of test/UI writes from delaying persistence +// indefinitely while waiting for a 2ms idle window. +const WRITER_BATCH_LIMIT: usize = 128; fn default_history_path() -> Option { dirs::home_dir().map(|home| home.join(".deepseek").join(HISTORY_FILE_NAME)) @@ -114,7 +117,7 @@ fn writer_sender() -> &'static Sender<(PathBuf, String)> { fn append_history_batch(rx: &Receiver<(PathBuf, String)>, first: (PathBuf, String)) { let mut pending = vec![first]; - loop { + while pending.len() < WRITER_BATCH_LIMIT { match rx.recv_timeout(Duration::from_millis(2)) { Ok(next) => pending.push(next), Err(RecvTimeoutError::Timeout) => break, diff --git a/crates/tui/src/config.rs b/crates/tui/src/config.rs index b41712557..f4a769bc9 100644 --- a/crates/tui/src/config.rs +++ b/crates/tui/src/config.rs @@ -8,6 +8,7 @@ use std::io::Write as _; use std::path::{Path, PathBuf}; use anyhow::{Context, Result}; +use codewhale_execpolicy::{ExecPolicyEngine, Ruleset, ToolPermissionRule}; use serde::{Deserialize, Serialize}; use serde_json::json; #[cfg(unix)] @@ -913,6 +914,12 @@ pub struct AutoConfig { pub cost_saving: Option, } +#[derive(Debug, Clone, Deserialize, Default)] +pub struct PermissionsConfig { + #[serde(default)] + pub rules: Vec, +} + /// Resolved CLI configuration, including defaults and environment overrides. #[derive(Debug, Clone, Default, Deserialize)] pub struct Config { @@ -947,6 +954,15 @@ pub struct Config { pub approval_policy: Option, pub sandbox_mode: Option, pub yolo: Option, + /// Legacy shell command permission rules. These are translated into typed + /// `exec_shell` allow/deny rules before tool execution. + #[serde(default)] + pub auto_allow: Vec, + #[serde(default)] + pub auto_deny: Vec, + /// Typed, persistent tool permission rules. + #[serde(default)] + pub permissions: PermissionsConfig, /// External sandbox backend: `"none"` or `"opensandbox"`. /// When set, exec_shell routes commands through the backend's HTTP API /// instead of spawning a local process. @@ -1818,6 +1834,20 @@ impl Config { self.allow_shell.unwrap_or(false) } + /// Build the user-layer permission ruleset used by the tool execution + /// approval path. + #[must_use] + pub fn permission_ruleset(&self) -> Ruleset { + Ruleset::user(self.auto_allow.clone(), self.auto_deny.clone()) + .with_rules(self.permissions.rules.clone()) + } + + /// Build the permission engine for shell/file tool approval decisions. + #[must_use] + pub fn exec_policy_engine(&self) -> ExecPolicyEngine { + ExecPolicyEngine::with_rulesets(vec![self.permission_ruleset()]) + } + /// Return the maximum number of concurrent sub-agents. /// Checks `[subagents] max_concurrent` first, then top-level `max_subagents`, /// then falls back to `DEFAULT_MAX_SUBAGENTS`. @@ -2917,6 +2947,9 @@ fn merge_config(base: Config, override_cfg: Config) -> Config { yolo: override_cfg.yolo.or(base.yolo), approval_policy: override_cfg.approval_policy.or(base.approval_policy), sandbox_mode: override_cfg.sandbox_mode.or(base.sandbox_mode), + auto_allow: merge_non_empty_vec(base.auto_allow, override_cfg.auto_allow), + auto_deny: merge_non_empty_vec(base.auto_deny, override_cfg.auto_deny), + permissions: merge_permissions_config(base.permissions, override_cfg.permissions), sandbox_backend: override_cfg.sandbox_backend.or(base.sandbox_backend), sandbox_url: override_cfg.sandbox_url.or(base.sandbox_url), sandbox_api_key: override_cfg.sandbox_api_key.or(base.sandbox_api_key), @@ -2974,6 +3007,25 @@ fn merge_config(base: Config, override_cfg: Config) -> Config { } } +fn merge_non_empty_vec(base: Vec, override_cfg: Vec) -> Vec { + if override_cfg.is_empty() { + base + } else { + override_cfg + } +} + +fn merge_permissions_config( + base: PermissionsConfig, + override_cfg: PermissionsConfig, +) -> PermissionsConfig { + if override_cfg.rules.is_empty() { + base + } else { + override_cfg + } +} + fn merge_provider_config(base: ProviderConfig, override_cfg: ProviderConfig) -> ProviderConfig { ProviderConfig { api_key: override_cfg.api_key.or(base.api_key), @@ -4046,6 +4098,44 @@ mod tests { ); } + #[test] + fn config_loads_typed_permission_rules_for_exec_policy_engine() -> Result<()> { + let _guard = lock_test_env(); + let temp = tempfile::tempdir()?; + let _env = EnvGuard::new(temp.path()); + let config_path = temp.path().join("config.toml"); + fs::write( + &config_path, + r#" +auto_allow = ["git status"] +auto_deny = ["rm -rf"] + +[[permissions.rules]] +tool = "read_file" +decision = "ask" +path = "secrets/**" +"#, + )?; + + let config = Config::load(Some(config_path), None)?; + assert_eq!(config.auto_allow, ["git status"]); + assert_eq!(config.auto_deny, ["rm -rf"]); + + let decision = config.exec_policy_engine().check_tool_permission( + codewhale_execpolicy::ToolPermissionContext { + tool: "read_file", + command: None, + path: Some("secrets/token.txt"), + }, + ); + + assert_eq!( + decision.decision, + Some(codewhale_execpolicy::PermissionDecision::Ask) + ); + Ok(()) + } + #[test] fn save_api_key_writes_config_file_under_cfg_test() -> Result<()> { // `save_api_key` writes to the shared user config file. This diff --git a/crates/tui/src/core/engine.rs b/crates/tui/src/core/engine.rs index fc286d583..3379d8403 100644 --- a/crates/tui/src/core/engine.rs +++ b/crates/tui/src/core/engine.rs @@ -171,6 +171,9 @@ pub struct EngineConfig { /// once at engine construction, then threaded onto every /// `SubAgentRuntime` the engine builds (#1806, #1808). pub subagent_api_timeout: Duration, + /// Typed permission rules used before falling back to the legacy approval + /// prompt path for shell and file tools. + pub exec_policy_engine: codewhale_execpolicy::ExecPolicyEngine, } impl Default for EngineConfig { @@ -214,6 +217,7 @@ impl Default for EngineConfig { subagent_api_timeout: Duration::from_secs( crate::config::DEFAULT_SUBAGENT_API_TIMEOUT_SECS, ), + exec_policy_engine: codewhale_execpolicy::ExecPolicyEngine::default(), } } } @@ -1981,10 +1985,11 @@ use self::approval::{ApprovalDecision, ApprovalResult, UserInputDecision}; use self::dispatch::should_parallelize_tool_batch; use self::dispatch::{ ParallelToolResult, ParallelToolResultEntry, ToolExecGuard, ToolExecOutcome, - ToolExecutionBatch, ToolExecutionPlan, caller_allowed_for_tool, caller_type_for_tool_use, - final_tool_input, format_tool_error, mcp_tool_approval_description, mcp_tool_is_parallel_safe, - mcp_tool_is_read_only, parse_parallel_tool_calls, parse_tool_input, + ToolExecutionBatch, ToolExecutionPlan, ToolPermissionOverride, caller_allowed_for_tool, + caller_type_for_tool_use, final_tool_input, format_tool_error, mcp_tool_approval_description, + mcp_tool_is_parallel_safe, mcp_tool_is_read_only, parse_parallel_tool_calls, parse_tool_input, plan_tool_execution_batches, should_force_update_plan_first, should_stop_after_plan_tool, + tool_permission_override_for_call, }; use self::loop_guard::{AttemptDecision, LoopGuard, OutcomeDecision}; #[cfg(test)] diff --git a/crates/tui/src/core/engine/dispatch.rs b/crates/tui/src/core/engine/dispatch.rs index 335639c4e..08dd1caf3 100644 --- a/crates/tui/src/core/engine/dispatch.rs +++ b/crates/tui/src/core/engine/dispatch.rs @@ -15,13 +15,18 @@ //! All items are `pub(super)`-only: the public engine surface (Op/Event, //! `EngineHandle`, `spawn_engine`) stays in `core/engine.rs`. -use serde_json::json; +use codewhale_execpolicy::{ + ExecPolicyEngine, PermissionDecision, ToolPermissionCheck, ToolPermissionContext, +}; +use serde_json::{Value, json}; +use std::path::{Component, Path, PathBuf}; use crate::models::{Tool, ToolCaller}; use crate::tools::spec::{ToolError, ToolResult}; use crate::tui::app::AppMode; use super::ToolUseState; +use super::tool_catalog::MULTI_TOOL_PARALLEL_NAME; // === Types ============================================================ @@ -70,6 +75,14 @@ pub(super) struct ParallelToolResult { pub(super) results: Vec, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) enum ToolPermissionOverride { + Unmatched, + Allow { reason: String }, + Ask { reason: String }, + Deny { reason: String }, +} + // Hold the lock guard for the duration of a tool execution. // The inner guards are held for RAII purposes (dropped when the guard is dropped). pub(super) enum ToolExecGuard<'a> { @@ -99,6 +112,452 @@ pub(super) fn caller_allowed_for_tool( requested == "direct" } +// === Typed permission rules ========================================== + +pub(super) fn tool_permission_override_for_call( + engine: &ExecPolicyEngine, + tool_name: &str, + input: &Value, + workspace: &Path, +) -> ToolPermissionOverride { + if tool_name == MULTI_TOOL_PARALLEL_NAME { + return permission_override_for_parallel_call(engine, input, workspace); + } + + if is_shell_permission_tool(tool_name) { + let command = input.get("command").and_then(Value::as_str); + return permission_override_for_context(engine, tool_name, command, None); + } + + if !is_file_permission_tool(tool_name) { + return ToolPermissionOverride::Unmatched; + } + + let paths = permission_paths_for_file_tool(tool_name, input); + if paths.is_empty() { + return permission_override_for_context(engine, tool_name, None, None); + } + + let mut saw_allow = false; + let mut saw_unmatched = false; + let mut ask_reason: Option = None; + + for path in paths { + match permission_override_for_path(engine, tool_name, &path, workspace) { + ToolPermissionOverride::Deny { reason } => { + return ToolPermissionOverride::Deny { reason }; + } + ToolPermissionOverride::Ask { reason } => { + ask_reason.get_or_insert(reason); + } + ToolPermissionOverride::Allow { .. } => { + saw_allow = true; + } + ToolPermissionOverride::Unmatched => { + saw_unmatched = true; + } + } + } + + if let Some(reason) = ask_reason { + ToolPermissionOverride::Ask { reason } + } else if saw_allow && !saw_unmatched { + ToolPermissionOverride::Allow { + reason: format!("Tool '{tool_name}' allowed by permission rule"), + } + } else { + ToolPermissionOverride::Unmatched + } +} + +fn permission_override_for_parallel_call( + engine: &ExecPolicyEngine, + input: &Value, + workspace: &Path, +) -> ToolPermissionOverride { + let Ok(calls) = parse_parallel_tool_calls(input) else { + return ToolPermissionOverride::Unmatched; + }; + + let mut saw_allow = false; + let mut saw_unmatched = false; + let mut ask_reason: Option = None; + + for (nested_tool_name, nested_input) in calls { + match tool_permission_override_for_call(engine, &nested_tool_name, &nested_input, workspace) + { + ToolPermissionOverride::Deny { reason } => { + return ToolPermissionOverride::Deny { reason }; + } + ToolPermissionOverride::Ask { reason } => { + ask_reason.get_or_insert(reason); + } + ToolPermissionOverride::Allow { .. } => { + saw_allow = true; + } + ToolPermissionOverride::Unmatched => { + saw_unmatched = true; + } + } + } + + if let Some(reason) = ask_reason { + ToolPermissionOverride::Ask { reason } + } else if saw_allow && !saw_unmatched { + ToolPermissionOverride::Allow { + reason: format!("Tool '{MULTI_TOOL_PARALLEL_NAME}' allowed by permission rule"), + } + } else { + ToolPermissionOverride::Unmatched + } +} + +fn permission_override_for_path( + engine: &ExecPolicyEngine, + tool_name: &str, + path: &str, + workspace: &Path, +) -> ToolPermissionOverride { + let mut override_result = ToolPermissionOverride::Unmatched; + for candidate in permission_path_candidates(path, workspace) { + let next = permission_override_for_context(engine, tool_name, None, Some(&candidate)); + override_result = combine_permission_overrides(override_result, next); + if matches!(override_result, ToolPermissionOverride::Deny { .. }) { + break; + } + } + override_result +} + +fn permission_override_for_context( + engine: &ExecPolicyEngine, + tool_name: &str, + command: Option<&str>, + path: Option<&str>, +) -> ToolPermissionOverride { + let mut override_result = ToolPermissionOverride::Unmatched; + for policy_tool_name in + std::iter::once(tool_name).chain(permission_policy_tool_aliases(tool_name).iter().copied()) + { + let next = permission_override_from_check( + tool_name, + engine.check_tool_permission(ToolPermissionContext { + tool: policy_tool_name, + command, + path, + }), + ); + override_result = combine_permission_overrides(override_result, next); + if matches!(override_result, ToolPermissionOverride::Deny { .. }) { + break; + } + } + override_result +} + +fn combine_permission_overrides( + current: ToolPermissionOverride, + next: ToolPermissionOverride, +) -> ToolPermissionOverride { + match (current, next) { + (ToolPermissionOverride::Deny { reason }, _) => ToolPermissionOverride::Deny { reason }, + (_, ToolPermissionOverride::Deny { reason }) => ToolPermissionOverride::Deny { reason }, + (ToolPermissionOverride::Ask { reason }, _) => ToolPermissionOverride::Ask { reason }, + (_, ToolPermissionOverride::Ask { reason }) => ToolPermissionOverride::Ask { reason }, + (ToolPermissionOverride::Allow { reason }, _) => ToolPermissionOverride::Allow { reason }, + (_, ToolPermissionOverride::Allow { reason }) => ToolPermissionOverride::Allow { reason }, + (ToolPermissionOverride::Unmatched, ToolPermissionOverride::Unmatched) => { + ToolPermissionOverride::Unmatched + } + } +} + +fn permission_override_from_check( + tool_name: &str, + check: ToolPermissionCheck, +) -> ToolPermissionOverride { + let Some(decision) = check.decision else { + return ToolPermissionOverride::Unmatched; + }; + let label = check + .matched_rule + .as_ref() + .map(|matched| matched.rule.pattern_label()) + .unwrap_or_else(|| format!("tool '{tool_name}'")); + match decision { + PermissionDecision::Allow => ToolPermissionOverride::Allow { + reason: format!("Tool '{tool_name}' allowed by permission rule ({label})"), + }, + PermissionDecision::Ask => ToolPermissionOverride::Ask { + reason: format!("Approval required by permission rule ({label})"), + }, + PermissionDecision::Deny => ToolPermissionOverride::Deny { + reason: format!("Tool '{tool_name}' denied by permission rule ({label})"), + }, + } +} + +fn is_shell_permission_tool(tool_name: &str) -> bool { + matches!( + tool_name, + "exec_shell" + | "exec_shell_wait" + | "exec_shell_interact" + | "exec_shell_cancel" + | "exec_wait" + | "exec_interact" + ) +} + +fn is_file_permission_tool(tool_name: &str) -> bool { + matches!( + tool_name, + "read_file" | "write_file" | "edit_file" | "list_dir" | "apply_patch" + ) +} + +fn permission_policy_tool_aliases(tool_name: &str) -> &'static [&'static str] { + match tool_name { + "read_file" => &["file_read"], + "write_file" => &["file_write"], + "edit_file" => &["file_edit"], + _ => &[], + } +} + +fn permission_paths_for_file_tool(tool_name: &str, input: &Value) -> Vec { + match tool_name { + "read_file" | "write_file" | "edit_file" => input + .get("path") + .and_then(Value::as_str) + .map(|path| vec![path.to_string()]) + .unwrap_or_default(), + "list_dir" => vec![ + input + .get("path") + .and_then(Value::as_str) + .unwrap_or(".") + .to_string(), + ], + "apply_patch" => apply_patch_permission_paths(input), + _ => Vec::new(), + } +} + +fn apply_patch_permission_paths(input: &Value) -> Vec { + let mut paths = Vec::new(); + if let Some(path) = input.get("path").and_then(Value::as_str) { + push_unique_path(&mut paths, path); + } + for key in ["changes", "files"] { + if let Some(entries) = input.get(key).and_then(Value::as_array) { + for entry in entries { + if let Some(path) = entry.get("path").and_then(Value::as_str) { + push_unique_path(&mut paths, path); + } + } + } + } + if paths.is_empty() { + if let Some(patch) = input.get("patch").and_then(Value::as_str) { + for path in parse_unified_diff_permission_paths(patch) { + push_unique_path(&mut paths, &path); + } + } + } + paths +} + +fn parse_unified_diff_permission_paths(patch: &str) -> Vec { + let mut paths = Vec::new(); + let mut old_path: Option = None; + + for line in patch.lines() { + if let Some(stripped) = line.strip_prefix("--- ") { + old_path = normalize_diff_permission_path(stripped); + continue; + } + if let Some(stripped) = line.strip_prefix("+++ ") { + let new_path = normalize_diff_permission_path(stripped); + if let Some(path) = new_path.or_else(|| old_path.clone()) { + push_unique_path(&mut paths, &path); + } + old_path = None; + } + } + + paths +} + +fn normalize_diff_permission_path(raw: &str) -> Option { + let raw = raw.trim(); + if raw.is_empty() || raw == "/dev/null" || raw == "dev/null" { + return None; + } + let raw = raw + .strip_prefix("a/") + .or_else(|| raw.strip_prefix("b/")) + .unwrap_or(raw); + Some(raw.to_string()) +} + +fn push_unique_path(paths: &mut Vec, path: &str) { + if !paths.iter().any(|existing| existing == path) { + paths.push(path.to_string()); + } +} + +fn permission_path_candidates(path: &str, workspace: &Path) -> Vec { + let mut candidates = Vec::new(); + push_unique_path(&mut candidates, path); + + let workspace_bases = permission_workspace_bases(workspace); + let workspace_base = workspace_bases + .first() + .cloned() + .unwrap_or_else(|| normalize_permission_path(workspace)); + let raw_path = Path::new(path); + let candidate_path = if raw_path.is_absolute() { + raw_path.to_path_buf() + } else { + workspace_base.join(raw_path) + }; + let candidate_path = normalize_permission_path(&candidate_path); + + for workspace_base in &workspace_bases { + if let Some(relative) = workspace_relative_permission_path(&candidate_path, workspace_base) + { + push_unique_path(&mut candidates, &relative); + } + } + + if let Some(canonical_path) = canonicalize_permission_path(&candidate_path) { + for workspace_base in &workspace_bases { + if let Some(relative) = + workspace_relative_permission_path(&canonical_path, workspace_base) + { + push_unique_path(&mut candidates, &relative); + } + } + } + + candidates +} + +fn permission_workspace_bases(workspace: &Path) -> Vec { + let mut bases = Vec::new(); + let workspace = if workspace.is_absolute() { + workspace.to_path_buf() + } else { + std::env::current_dir() + .map(|current_dir| current_dir.join(workspace)) + .unwrap_or_else(|_| workspace.to_path_buf()) + }; + push_unique_path_buf(&mut bases, normalize_permission_path(&workspace)); + if let Ok(canonical) = workspace.canonicalize() { + push_unique_path_buf(&mut bases, normalize_permission_path(&canonical)); + } + bases +} + +fn workspace_relative_permission_path(path: &Path, workspace: &Path) -> Option { + let normalized_path = normalize_permission_path(path); + normalized_path + .strip_prefix(workspace) + .ok() + .map(permission_path_to_string) +} + +fn canonicalize_permission_path(path: &Path) -> Option { + if path.exists() { + return path + .canonicalize() + .ok() + .map(|path| normalize_permission_path(&path)); + } + + let mut existing_ancestor = path.to_path_buf(); + let mut suffix_parts: Vec = Vec::new(); + + while !existing_ancestor.exists() { + if let Some(file_name) = existing_ancestor.file_name() { + suffix_parts.push(file_name.to_owned()); + } + match existing_ancestor.parent() { + Some(parent) if !parent.as_os_str().is_empty() => { + existing_ancestor = parent.to_path_buf(); + } + _ => return None, + } + } + + let mut canonical = existing_ancestor + .canonicalize() + .unwrap_or(existing_ancestor); + for part in suffix_parts.into_iter().rev() { + canonical.push(part); + } + Some(normalize_permission_path(&canonical)) +} + +fn normalize_permission_path(path: &Path) -> PathBuf { + let mut prefix: Option = None; + let mut is_root = false; + let mut stack: Vec = Vec::new(); + + for component in path.components() { + match component { + Component::Prefix(prefix_component) => { + prefix = Some(prefix_component.as_os_str().to_owned()); + } + Component::RootDir => { + is_root = true; + } + Component::CurDir => {} + Component::ParentDir => { + let parent = Component::ParentDir.as_os_str(); + if let Some(last) = stack.pop() { + if last == parent { + stack.push(last); + stack.push(parent.to_owned()); + } + } else if !is_root { + stack.push(parent.to_owned()); + } + } + Component::Normal(part) => { + stack.push(part.to_owned()); + } + } + } + + let mut normalized = PathBuf::new(); + if let Some(prefix) = prefix { + normalized.push(prefix); + } + if is_root { + normalized.push(Path::new(std::path::MAIN_SEPARATOR_STR)); + } + for part in stack { + normalized.push(part); + } + normalized +} + +fn permission_path_to_string(path: &Path) -> String { + if path.as_os_str().is_empty() { + ".".to_string() + } else { + path.to_string_lossy().replace('\\', "/") + } +} + +fn push_unique_path_buf(paths: &mut Vec, path: PathBuf) { + if !paths.iter().any(|existing| existing == &path) { + paths.push(path); + } +} + pub(super) fn format_tool_error(err: &ToolError, tool_name: &str) -> String { match err { ToolError::InvalidInput { message } => { diff --git a/crates/tui/src/core/engine/tests.rs b/crates/tui/src/core/engine/tests.rs index 851b09ea0..2093c8e8e 100644 --- a/crates/tui/src/core/engine/tests.rs +++ b/crates/tui/src/core/engine/tests.rs @@ -152,6 +152,210 @@ fn make_plan_at( } } +fn permission_engine_with_rules( + rules: Vec, +) -> codewhale_execpolicy::ExecPolicyEngine { + codewhale_execpolicy::ExecPolicyEngine::with_rulesets(vec![ + codewhale_execpolicy::Ruleset::user(vec![], vec![]).with_rules(rules), + ]) +} + +fn permission_test_workspace() -> &'static Path { + Path::new(".") +} + +#[test] +fn typed_permission_allows_shell_command_before_approval_prompt() { + let engine = + permission_engine_with_rules(vec![codewhale_execpolicy::ToolPermissionRule::exec_shell( + codewhale_execpolicy::PermissionDecision::Allow, + "cargo test", + )]); + + let decision = tool_permission_override_for_call( + &engine, + "exec_shell", + &json!({"command": "cargo test --workspace"}), + permission_test_workspace(), + ); + + assert!(matches!(decision, ToolPermissionOverride::Allow { .. })); +} + +#[test] +fn typed_permission_can_force_read_file_approval() { + let engine = + permission_engine_with_rules(vec![codewhale_execpolicy::ToolPermissionRule::file_path( + "read_file", + codewhale_execpolicy::PermissionDecision::Ask, + "secrets/**", + )]); + + let decision = tool_permission_override_for_call( + &engine, + "read_file", + &json!({"path": "secrets/a.env"}), + permission_test_workspace(), + ); + + assert!(matches!(decision, ToolPermissionOverride::Ask { .. })); +} + +#[test] +fn typed_permission_matches_absolute_workspace_file_paths() { + let workspace = tempdir().expect("workspace"); + let absolute_path = workspace.path().join("secrets/a.env"); + let engine = + permission_engine_with_rules(vec![codewhale_execpolicy::ToolPermissionRule::file_path( + "read_file", + codewhale_execpolicy::PermissionDecision::Deny, + "secrets/**", + )]); + + let decision = tool_permission_override_for_call( + &engine, + "read_file", + &json!({"path": absolute_path.to_string_lossy()}), + workspace.path(), + ); + + assert!(matches!(decision, ToolPermissionOverride::Deny { .. })); +} + +#[cfg(unix)] +#[test] +fn typed_permission_matches_symlink_resolved_workspace_file_paths() { + use std::os::unix::fs::symlink; + + let workspace = tempdir().expect("workspace"); + let secrets_dir = workspace.path().join("secrets"); + fs::create_dir(&secrets_dir).expect("create secrets dir"); + fs::write(secrets_dir.join("a.env"), "token").expect("write secret"); + symlink(&secrets_dir, workspace.path().join("alias")).expect("symlink secrets"); + let engine = + permission_engine_with_rules(vec![codewhale_execpolicy::ToolPermissionRule::file_path( + "read_file", + codewhale_execpolicy::PermissionDecision::Deny, + "secrets/**", + )]); + + let decision = tool_permission_override_for_call( + &engine, + "read_file", + &json!({"path": "alias/a.env"}), + workspace.path(), + ); + + assert!(matches!(decision, ToolPermissionOverride::Deny { .. })); +} + +#[test] +fn typed_permission_checks_nested_parallel_tool_calls() { + let engine = + permission_engine_with_rules(vec![codewhale_execpolicy::ToolPermissionRule::file_path( + "read_file", + codewhale_execpolicy::PermissionDecision::Deny, + "secrets/**", + )]); + + let decision = tool_permission_override_for_call( + &engine, + "multi_tool_use.parallel", + &json!({ + "tool_uses": [{ + "recipient_name": "read_file", + "parameters": { "path": "secrets/a.env" } + }] + }), + permission_test_workspace(), + ); + + assert!(matches!(decision, ToolPermissionOverride::Deny { .. })); +} + +#[test] +fn typed_permission_matches_legacy_file_tool_aliases() { + let engine = + permission_engine_with_rules(vec![codewhale_execpolicy::ToolPermissionRule::file_path( + "file_edit", + codewhale_execpolicy::PermissionDecision::Ask, + "src/**", + )]); + + let decision = tool_permission_override_for_call( + &engine, + "edit_file", + &json!({"path": "src/main.rs"}), + permission_test_workspace(), + ); + + assert!(matches!(decision, ToolPermissionOverride::Ask { .. })); +} + +#[test] +fn typed_permission_denies_file_path_before_tool_execution() { + let engine = + permission_engine_with_rules(vec![codewhale_execpolicy::ToolPermissionRule::file_path( + "write_file", + codewhale_execpolicy::PermissionDecision::Deny, + "src/**", + )]); + + let decision = tool_permission_override_for_call( + &engine, + "write_file", + &json!({"path": "src/main.rs", "content": "fn main() {}"}), + permission_test_workspace(), + ); + + assert!(matches!(decision, ToolPermissionOverride::Deny { .. })); +} + +#[test] +fn typed_permission_requires_all_apply_patch_paths_to_be_allowed() { + let engine = + permission_engine_with_rules(vec![codewhale_execpolicy::ToolPermissionRule::file_path( + "apply_patch", + codewhale_execpolicy::PermissionDecision::Allow, + "docs/**", + )]); + + let decision = tool_permission_override_for_call( + &engine, + "apply_patch", + &json!({ + "changes": [ + { "path": "docs/a.md", "content": "ok" }, + { "path": "src/lib.rs", "content": "needs existing approval" } + ] + }), + permission_test_workspace(), + ); + + assert_eq!(decision, ToolPermissionOverride::Unmatched); +} + +#[test] +fn typed_permission_allows_apply_patch_when_all_paths_match() { + let engine = + permission_engine_with_rules(vec![codewhale_execpolicy::ToolPermissionRule::file_path( + "apply_patch", + codewhale_execpolicy::PermissionDecision::Allow, + "docs/**", + )]); + + let decision = tool_permission_override_for_call( + &engine, + "apply_patch", + &json!({ + "patch": "--- a/docs/a.md\n+++ b/docs/a.md\n@@ -1 +1 @@\n-old\n+new\n" + }), + permission_test_workspace(), + ); + + assert!(matches!(decision, ToolPermissionOverride::Allow { .. })); +} + 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 9f2da5ffd..7fa05c39c 100644 --- a/crates/tui/src/core/engine/turn_loop.rs +++ b/crates/tui/src/core/engine/turn_loop.rs @@ -1239,6 +1239,7 @@ impl Engine { if blocked_error.is_none() && tool_def.is_none() && !McpPool::is_mcp_tool(&tool_name) + && tool_name != MULTI_TOOL_PARALLEL_NAME && tool_name != CODE_EXECUTION_TOOL_NAME && tool_name != JS_EXECUTION_TOOL_NAME && !is_tool_search_tool(&tool_name) @@ -1261,6 +1262,12 @@ impl Engine { approval_description = spec.description().to_string(); supports_parallel = spec.supports_parallel(); read_only = spec.is_read_only(); + } else if tool_name == MULTI_TOOL_PARALLEL_NAME { + approval_required = false; + approval_description = + "Execute multiple read-only tools in parallel".to_string(); + supports_parallel = false; + read_only = true; } else if tool_name == CODE_EXECUTION_TOOL_NAME { approval_required = true; approval_description = @@ -1283,26 +1290,52 @@ impl Engine { let should_emit_hydration_status = !deferred_tools_hydrated_this_batch.contains(&tool_name); - if blocked_error.is_none() - && let Some(result) = maybe_hydrate_requested_deferred_tool( + + if blocked_error.is_none() { + match tool_permission_override_for_call( + &self.config.exec_policy_engine, + &tool_name, + &tool_input, + &self.session.workspace, + ) { + ToolPermissionOverride::Allow { reason } => { + approval_required = false; + approval_description = reason; + } + ToolPermissionOverride::Ask { reason } => { + approval_required = true; + approval_description = reason; + } + ToolPermissionOverride::Deny { reason } => { + approval_required = false; + blocked_error = Some(ToolError::permission_denied(reason)); + } + ToolPermissionOverride::Unmatched => {} + } + } + + if blocked_error.is_none() { + if let Some(result) = maybe_hydrate_requested_deferred_tool( &tool_name, &tool_input, &tool_catalog, &active_tools_at_batch_start, &mut deferred_tools_hydrated_this_batch, - ) - { - if should_emit_hydration_status { - let status = if requested_tool_name == tool_name { - format!("Auto-loaded deferred tool '{tool_name}' after model request.") - } else { - format!( - "Auto-loaded deferred tool '{tool_name}' after resolving '{requested_tool_name}'." - ) - }; - let _ = self.tx_event.send(Event::status(status)).await; + ) { + if should_emit_hydration_status { + let status = if requested_tool_name == tool_name { + format!( + "Auto-loaded deferred tool '{tool_name}' after model request." + ) + } else { + format!( + "Auto-loaded deferred tool '{tool_name}' after resolving '{requested_tool_name}'." + ) + }; + let _ = self.tx_event.send(Event::status(status)).await; + } + guard_result = Some(result); } - guard_result = Some(result); } if blocked_error.is_none() diff --git a/crates/tui/src/main.rs b/crates/tui/src/main.rs index 34cb7fce7..77d6aa12a 100644 --- a/crates/tui/src/main.rs +++ b/crates/tui/src/main.rs @@ -8,6 +8,7 @@ use std::time::Duration; use anyhow::{Context, Result, anyhow, bail}; use clap::{Args, CommandFactory, Parser, Subcommand, ValueEnum}; use clap_complete::{Shell, generate}; +use codewhale_execpolicy::PermissionDecision; use dotenvy::dotenv; use tempfile::NamedTempFile; use wait_timeout::ChildExt; @@ -4696,6 +4697,11 @@ fn merge_project_config(config: &mut Config, workspace: &Path) { config.allow_shell = Some(v); } + // Project-level permission config is allowed only when it tightens the + // effective policy. A repository can require extra prompts or denials, but + // it cannot grant itself broader tool access. + merge_project_permission_config(config, table); + // #454: instructions array — project replaces user. Empty arrays // count: explicit `instructions = []` clears the user's list for // this repo, useful when the user has a verbose global file that @@ -4711,6 +4717,72 @@ fn merge_project_config(config: &mut Config, workspace: &Path) { } } +fn merge_project_permission_config( + config: &mut Config, + table: &toml::map::Map, +) { + if let Some(value) = table.get("auto_allow") { + let ignored = value.as_array().map_or(0, Vec::len); + if ignored > 0 { + eprintln!( + "warning: project-scope `auto_allow` is ignored — \ + project config cannot grant persistent command allow rules." + ); + } + } + + if let Some(value) = table.get("auto_deny") { + match string_array_from_project_value(value) { + Some(entries) => config.auto_deny.extend(entries), + None => eprintln!( + "warning: project-scope `auto_deny` is ignored — expected an array of strings." + ), + } + } + + let Some(value) = table.get("permissions") else { + return; + }; + let permissions: crate::config::PermissionsConfig = match value.clone().try_into() { + Ok(permissions) => permissions, + Err(err) => { + eprintln!( + "warning: project-scope `[permissions]` is ignored — failed to parse permission rules: {err}" + ); + return; + } + }; + + for rule in permissions.rules { + match rule.decision { + PermissionDecision::Deny | PermissionDecision::Ask => { + config.permissions.rules.push(rule); + } + PermissionDecision::Allow => { + eprintln!( + "warning: project-scope allow permission rule `{}` is ignored — \ + project config cannot grant persistent tool allow rules.", + rule.pattern_label() + ); + } + } + } +} + +fn string_array_from_project_value(value: &toml::Value) -> Option> { + Some( + value + .as_array()? + .iter() + .map(toml::Value::as_str) + .collect::>>()? + .into_iter() + .filter(|item| !item.trim().is_empty()) + .map(str::to_string) + .collect(), + ) +} + async fn run_interactive( cli: &Cli, config: &Config, @@ -5136,6 +5208,7 @@ async fn run_exec_agent( .and_then(|s| s.provider) .unwrap_or_default(), search_api_key: config.search.as_ref().and_then(|s| s.api_key.clone()), + exec_policy_engine: config.exec_policy_engine(), }; let engine_handle = spawn_engine(engine_config, config); @@ -6303,6 +6376,48 @@ allow_shell = false assert_eq!(config.allow_shell, Some(false)); } + #[test] + fn project_overlay_applies_only_tightening_permission_rules() { + let tmp = workspace_with_project_config( + r#" +auto_allow = ["cargo test"] +auto_deny = ["rm -rf"] + +[[permissions.rules]] +tool = "read_file" +decision = "ask" +path = "secrets/**" + +[[permissions.rules]] +tool = "write_file" +decision = "deny" +path = "src/**" + +[[permissions.rules]] +tool = "exec_shell" +decision = "allow" +command = "cargo test" +"#, + ); + let mut config = Config::default(); + merge_project_config(&mut config, tmp.path()); + + assert!( + config.auto_allow.is_empty(), + "project-scope auto_allow must not grant access" + ); + assert_eq!(config.auto_deny, ["rm -rf"]); + assert_eq!(config.permissions.rules.len(), 2); + assert_eq!( + config.permissions.rules[0].decision, + PermissionDecision::Ask + ); + assert_eq!( + config.permissions.rules[1].decision, + PermissionDecision::Deny + ); + } + #[test] fn project_overlay_clamps_max_subagents_to_safe_range() { let tmp = workspace_with_project_config( diff --git a/crates/tui/src/runtime_threads.rs b/crates/tui/src/runtime_threads.rs index 787142ba4..824a198a3 100644 --- a/crates/tui/src/runtime_threads.rs +++ b/crates/tui/src/runtime_threads.rs @@ -1985,6 +1985,7 @@ impl RuntimeThreadManager { .and_then(|s| s.provider) .unwrap_or_default(), search_api_key: self.config.search.as_ref().and_then(|s| s.api_key.clone()), + exec_policy_engine: self.config.exec_policy_engine(), }; let engine = spawn_engine(engine_cfg, &self.config); diff --git a/crates/tui/src/tools/pandoc.rs b/crates/tui/src/tools/pandoc.rs index c8a5b78d7..6af3cb4bd 100644 --- a/crates/tui/src/tools/pandoc.rs +++ b/crates/tui/src/tools/pandoc.rs @@ -144,7 +144,9 @@ impl ToolSpec for PandocConvertTool { // gated on resolve_pandoc(), but a concurrent uninstall between // catalog build and the model's call should produce a clear // error rather than the cryptic "program not found" from raw - // Command::spawn. + // Command::spawn. Input validation above intentionally runs first + // so tests and users get deterministic schema/actionable errors + // even on machines without pandoc installed. let pandoc = crate::dependencies::resolve_pandoc().ok_or_else(|| { ToolError::execution_failed( "pandoc_convert: pandoc binary not found on PATH. \ diff --git a/crates/tui/src/tui/sidebar.rs b/crates/tui/src/tui/sidebar.rs index bff5c51a0..5f557dd78 100644 --- a/crates/tui/src/tui/sidebar.rs +++ b/crates/tui/src/tui/sidebar.rs @@ -1851,8 +1851,8 @@ mod tests { use super::{ ACTIVE_TOOL_COMPLETED_ROW_TTL, ACTIVE_TOOL_STALE_RUNNING_ROW_TTL, AutoSidebarPanel, AutoSidebarState, SidebarAgentRow, SidebarSubagentSummary, SidebarWorkChecklistItem, - SidebarWorkStrategyStep, SidebarWorkSummary, auto_sidebar_panels, subagent_panel_lines, - task_panel_lines, work_panel_empty_hint, work_panel_lines, + SidebarWorkStrategyStep, SidebarWorkSummary, auto_sidebar_panels, duration_ms, + subagent_panel_lines, task_panel_lines, work_panel_empty_hint, work_panel_lines, }; use crate::config::Config; use crate::palette::PaletteMode; @@ -2195,6 +2195,8 @@ mod tests { fn tasks_panel_collapses_stale_running_tool_rows() { let mut app = create_test_app(); let mut active = ActiveCell::new(); + let stale_duration_ms = + duration_ms(ACTIVE_TOOL_STALE_RUNNING_ROW_TTL).saturating_add(1_000); for (idx, command) in ["long one", "long two"].into_iter().enumerate() { active.push_tool( format!("shell-{idx}"), @@ -2203,7 +2205,7 @@ mod tests { status: ToolStatus::Running, output: None, started_at: None, - duration_ms: Some(ACTIVE_TOOL_STALE_RUNNING_ROW_TTL.as_millis() as u64 + 1), + duration_ms: Some(stale_duration_ms), source: ExecSource::Assistant, interaction: None, output_summary: None, diff --git a/crates/tui/src/tui/ui.rs b/crates/tui/src/tui/ui.rs index 1444f1341..672369427 100644 --- a/crates/tui/src/tui/ui.rs +++ b/crates/tui/src/tui/ui.rs @@ -723,6 +723,7 @@ fn build_engine_config(app: &App, config: &Config) -> EngineConfig { .and_then(|s| s.provider) .unwrap_or_default(), search_api_key: config.search.as_ref().and_then(|s| s.api_key.clone()), + exec_policy_engine: config.exec_policy_engine(), } } diff --git a/crates/tui/src/vision/tools.rs b/crates/tui/src/vision/tools.rs index bfce551df..d7ff37e61 100644 --- a/crates/tui/src/vision/tools.rs +++ b/crates/tui/src/vision/tools.rs @@ -270,13 +270,13 @@ mod tests { let tmp = tempdir().expect("tempdir"); let ctx = ToolContext::new(tmp.path().to_path_buf()); let tool = ImageAnalyzeTool::new(fake_config()); - let outside_workspace = if cfg!(windows) { + let absolute_path = if cfg!(windows) { r"C:\Windows\System32\drivers\etc\hosts" } else { "/etc/hosts" }; let err = tool - .execute(json!({"image_path": outside_workspace}), &ctx) + .execute(json!({"image_path": absolute_path}), &ctx) .await .expect_err("absolute path must reject"); assert!(