Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 87 additions & 11 deletions crates/tui/src/core/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1046,12 +1046,12 @@ impl Engine {
&self.session.workspace,
self.session.approval_mode,
);
if let Some(ExecShellAskRuleDecision::Prompt(reason)) = ask_rule_decision.as_ref() {
if let Some(ToolAskRuleDecision::Prompt(reason)) = ask_rule_decision.as_ref() {
approval_required = true;
approval_description = reason.clone();
approval_force_prompt = true;
}
if let Some(ExecShellAskRuleDecision::Block(reason)) = ask_rule_decision {
if let Some(ToolAskRuleDecision::Block(reason)) = ask_rule_decision {
Err(ToolError::permission_denied(reason))
} else if approval_required {
emit_tool_audit(json!({
Expand Down Expand Up @@ -3315,7 +3315,7 @@ fn agent_approval_mode_for_turn(
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub(super) enum ExecShellAskRuleDecision {
pub(super) enum ToolAskRuleDecision {
Prompt(String),
Block(String),
}
Expand All @@ -3326,11 +3326,63 @@ pub(super) fn exec_shell_ask_rule_decision(
tool_input: &Value,
workspace: &Path,
approval_mode: crate::tui::approval::ApprovalMode,
) -> Option<ExecShellAskRuleDecision> {
) -> Option<ToolAskRuleDecision> {
if tool_name != "exec_shell" {
return None;
}
let command = tool_input.get("command").and_then(Value::as_str)?;
tool_ask_rule_decision_for_context(config, tool_name, command, None, workspace, approval_mode)
}

pub(super) fn file_tool_ask_rule_decision(
config: &EngineConfig,
tool_name: &str,
tool_input: &Value,
workspace: &Path,
approval_mode: crate::tui::approval::ApprovalMode,
) -> Option<ToolAskRuleDecision> {
let paths = file_tool_permission_paths(tool_name, tool_input)?;
if paths.is_empty() {
return tool_ask_rule_decision_for_context(
config,
tool_name,
"",
None,
workspace,
approval_mode,
);
}

let mut prompt: Option<String> = None;
for path in paths {
match tool_ask_rule_decision_for_context(
config,
tool_name,
"",
Some(&path),
workspace,
approval_mode,
) {
Some(ToolAskRuleDecision::Block(reason)) => {
return Some(ToolAskRuleDecision::Block(reason));
}
Some(ToolAskRuleDecision::Prompt(reason)) => {
prompt.get_or_insert(reason);
}
None => {}
}
}
prompt.map(ToolAskRuleDecision::Prompt)
}

fn tool_ask_rule_decision_for_context(
config: &EngineConfig,
tool_name: &str,
command: &str,
path: Option<&str>,
workspace: &Path,
approval_mode: crate::tui::approval::ApprovalMode,
) -> Option<ToolAskRuleDecision> {
let cwd = workspace.to_string_lossy();
let ask_for_approval = match approval_mode {
crate::tui::approval::ApprovalMode::Never => AskForApproval::Never,
Expand All @@ -3344,24 +3396,48 @@ pub(super) fn exec_shell_ask_rule_decision(
command,
cwd: cwd.as_ref(),
tool: Some(tool_name),
path: None,
path,
ask_for_approval,
sandbox_mode: None,
})
.ok()?;
if !decision.allow {
Some(ExecShellAskRuleDecision::Block(
decision.reason().to_string(),
))
Some(ToolAskRuleDecision::Block(decision.reason().to_string()))
} else if decision.requires_approval {
Some(ExecShellAskRuleDecision::Prompt(
decision.reason().to_string(),
))
Some(ToolAskRuleDecision::Prompt(decision.reason().to_string()))
} else {
None
}
}

fn file_tool_permission_paths(tool_name: &str, input: &Value) -> Option<Vec<String>> {
match tool_name {
"read_file" | "write_file" | "edit_file" | "file_search" | "grep_files" => {
Some(string_field(input, "path").into_iter().collect())
}
"list_dir" => Some(vec![
string_field(input, "path").unwrap_or_else(|| ".".to_string()),
]),
"apply_patch" => Some(apply_patch_permission_paths(input)),
_ => None,
}
}

fn string_field(input: &Value, key: &str) -> Option<String> {
input
.get(key)
.and_then(Value::as_str)
.map(str::trim)
.filter(|value| !value.is_empty())
.map(str::to_string)
}

fn apply_patch_permission_paths(input: &Value) -> Vec<String> {
crate::tools::apply_patch::preflight_apply_patch(input)
.map(|preflight| preflight.touched_files)
.unwrap_or_default()
}

/// Spawn the engine in a background task
pub fn spawn_engine(config: EngineConfig, api_config: &Config) -> EngineHandle {
let (engine, handle) = Engine::new(config, api_config);
Expand Down
77 changes: 75 additions & 2 deletions crates/tui/src/core/engine/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,14 @@ fn ask_rule_engine(command: &str) -> codewhale_execpolicy::ExecPolicyEngine {
])
}

fn file_ask_rule_engine(tool: &str, path: &str) -> codewhale_execpolicy::ExecPolicyEngine {
codewhale_execpolicy::ExecPolicyEngine::with_rulesets(vec![
codewhale_execpolicy::Ruleset::user(vec![], vec![]).with_ask_rules(vec![
codewhale_execpolicy::ToolAskRule::file_path(tool, path),
]),
])
}

#[test]
fn exec_shell_ask_rule_decision_prompts_for_matching_auto_command() {
let config = EngineConfig {
Expand All @@ -312,7 +320,7 @@ fn exec_shell_ask_rule_decision_prompts_for_matching_auto_command() {

assert_eq!(
decision,
Some(ExecShellAskRuleDecision::Prompt(
Some(ToolAskRuleDecision::Prompt(
"Typed ask rule 'tool=exec_shell command=cargo test' requires approval.".to_string()
))
);
Expand All @@ -335,7 +343,7 @@ fn exec_shell_ask_rule_decision_blocks_matching_never_command() {

assert_eq!(
decision,
Some(ExecShellAskRuleDecision::Block(
Some(ToolAskRuleDecision::Block(
"Typed ask rule 'tool=exec_shell command=cargo test' requires approval, but approval policy is never.".to_string()
))
);
Expand All @@ -359,6 +367,71 @@ fn exec_shell_ask_rule_decision_ignores_unmatched_command() {
assert_eq!(decision, None);
}

#[test]
fn file_ask_rule_decision_prompts_for_matching_read_path() {
let config = EngineConfig {
exec_policy_engine: file_ask_rule_engine("read_file", "secrets/api_key.txt"),
..EngineConfig::default()
};

let decision = file_tool_ask_rule_decision(
&config,
"read_file",
&json!({"path": "secrets/api_key.txt"}),
Path::new("/repo"),
crate::tui::approval::ApprovalMode::Auto,
);

assert_eq!(
decision,
Some(ToolAskRuleDecision::Prompt(
"Typed ask rule 'tool=read_file path=secrets/api_key.txt' requires approval."
.to_string()
))
);
}

#[test]
fn file_ask_rule_decision_blocks_matching_read_path_when_approval_is_never() {
let config = EngineConfig {
exec_policy_engine: file_ask_rule_engine("read_file", "secrets/api_key.txt"),
..EngineConfig::default()
};

let decision = file_tool_ask_rule_decision(
&config,
"read_file",
&json!({"path": "secrets/api_key.txt"}),
Path::new("/repo"),
crate::tui::approval::ApprovalMode::Never,
);

assert_eq!(
decision,
Some(ToolAskRuleDecision::Block(
"Typed ask rule 'tool=read_file path=secrets/api_key.txt' requires approval, but approval policy is never.".to_string()
))
);
}

#[test]
fn file_ask_rule_decision_ignores_unmatched_path() {
let config = EngineConfig {
exec_policy_engine: file_ask_rule_engine("read_file", "secrets/api_key.txt"),
..EngineConfig::default()
};

let decision = file_tool_ask_rule_decision(
&config,
"read_file",
&json!({"path": "docs/readme.md"}),
Path::new("/repo"),
crate::tui::approval::ApprovalMode::Auto,
);

assert_eq!(decision, None);
}

fn api_tool(name: &str) -> Tool {
Tool {
tool_type: Some("function".to_string()),
Expand Down
18 changes: 14 additions & 4 deletions crates/tui/src/core/engine/turn_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1582,22 +1582,32 @@ impl Engine {
approval_required = true;
}

if blocked_error.is_none()
&& let Some(decision) = exec_shell_ask_rule_decision(
let ask_rule_decision = exec_shell_ask_rule_decision(
&self.config,
&tool_name,
&tool_input,
&self.session.workspace,
self.session.approval_mode,
)
.or_else(|| {
file_tool_ask_rule_decision(
&self.config,
&tool_name,
&tool_input,
&self.session.workspace,
self.session.approval_mode,
)
});
if blocked_error.is_none()
&& let Some(decision) = ask_rule_decision
{
Comment on lines +1585 to 1603

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Evaluating the ask rules (exec_shell_ask_rule_decision and file_tool_ask_rule_decision) can be expensive, especially for apply_patch which parses unified diffs. We should only perform these checks if blocked_error is None to avoid unnecessary work when the tool is already blocked.

                if blocked_error.is_none() {
                    let ask_rule_decision = exec_shell_ask_rule_decision(
                        &self.config,
                        &tool_name,
                        &tool_input,
                        &self.session.workspace,
                        self.session.approval_mode,
                    )
                    .or_else(|| {
                        file_tool_ask_rule_decision(
                            &self.config,
                            &tool_name,
                            &tool_input,
                            &self.session.workspace,
                            self.session.approval_mode,
                        )
                    });
                    if let Some(decision) = ask_rule_decision {
                        match decision {
                            ToolAskRuleDecision::Prompt(reason) => {
                                approval_required = true;
                                approval_description = reason;
                                approval_force_prompt = true;
                            }
                            ToolAskRuleDecision::Block(reason) => {
                                approval_required = false;
                                approval_force_prompt = false;
                                blocked_error = Some(ToolError::permission_denied(reason));
                            }
                        }
                    }
                }

match decision {
ExecShellAskRuleDecision::Prompt(reason) => {
ToolAskRuleDecision::Prompt(reason) => {
approval_required = true;
approval_description = reason;
approval_force_prompt = true;
}
ExecShellAskRuleDecision::Block(reason) => {
ToolAskRuleDecision::Block(reason) => {
approval_required = false;
approval_force_prompt = false;
blocked_error = Some(ToolError::permission_denied(reason));
Expand Down
9 changes: 5 additions & 4 deletions docs/TOOL_SURFACE.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,11 @@ Shell permission policy is evaluated by `crates/execpolicy`. Deny prefixes are
checked before trusted prefixes and block matching commands regardless of layer.
Trusted prefixes only skip approval in modes that permit trust shortcuts. Typed
ask records are currently a narrow foundation: when one matches under
`AskForApproval::Never`, the command is rejected because the runtime cannot ask
the user; existing allow/deny behavior is otherwise unchanged. The TUI runtime
loads ask-only records from the sibling `permissions.toml` file and applies
matching `exec_shell` command ask-rules before Auto/session approval shortcuts.
`AskForApproval::Never`, the invocation is rejected because the runtime cannot
ask the user; existing allow/deny behavior is otherwise unchanged. The TUI
runtime loads ask-only records from the sibling `permissions.toml` file and
applies matching `exec_shell` command ask-rules and explicit file-path ask-rules
before Auto/session approval shortcuts.

### MCP manager and palette discovery

Expand Down