Skip to content

Commit c4d02aa

Browse files
author
Yogthos
committed
fix(perm): session allowlist matches absolute paths, suggest_pattern covers all tools, better dialog
HIGH fixes: - checker.rs: resolve absolute path BEFORE is_session_allowed and check both raw+absolute forms. Fixes bug where AllowAlways on a relative path (src/main.rs) didn't match subsequent absolute-path calls (/Users/.../src/main.rs) to the same file. - ui/mod.rs: add suggest_pattern arms for all 20+ previously-missing tools (apply_patch, semantic tools, webfetch, websearch, task, glob, repo_overview, etc.). These tools previously got placeholder pattern causing AllowAlways to silently degrade to AllowOnce. MEDIUM fixes: - ui/mod.rs: show path context in permission dialog. File tools now show 'path: src/main.rs (inside project)' instead of generic 'args: 1'. Bash shows 'command:', MCP shows 'mcp:', etc. - checker.rs: make resolve_absolute pub(crate) for dialog use. LOW fix: - checker.rs: add find_definition/find_callers to is_path_tool_name so their rules use path-glob semantics (matches check_perm_path).
1 parent 2c920ab commit c4d02aa

2 files changed

Lines changed: 80 additions & 15 deletions

File tree

src/permission/checker.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ pub(crate) fn is_path_tool_name(tool: &str) -> bool {
8484
// Semantic tools whose primary arg is a file path.
8585
| "list_symbols"
8686
| "get_symbol_body"
87+
| "find_definition"
88+
| "find_callers"
8789
| "find_callees"
8890
// #1 fix: repo_overview's arg is a directory path; user
8991
// rules like `"/etc/**": "deny"` need path-glob semantics
@@ -494,11 +496,17 @@ impl PermissionChecker {
494496
return CheckResult::Allowed;
495497
}
496498

497-
if self.is_session_allowed(tool, path) {
499+
// Resolve BEFORE the allowlist check so we can test both the
500+
// raw path and the absolute form. Without this, a user who
501+
// granted AllowAlways for a relative path (e.g. src/main.rs)
502+
// gets re-prompted when the LLM sends an absolute path for
503+
// the same file.
504+
let abs_path = resolve_absolute(path, &self.working_dir);
505+
506+
if self.is_session_allowed(tool, path) || self.is_session_allowed(tool, &abs_path) {
498507
return CheckResult::Allowed;
499508
}
500509

501-
let abs_path = resolve_absolute(path, &self.working_dir);
502510
let mut matched: Vec<(Action, String)> = Vec::new();
503511
if let Some(rules) = self.rules.get(tool) {
504512
for (pattern, action) in rules {
@@ -855,7 +863,7 @@ fn install_cwd_allow_rules(
855863
Some(cwd_glob)
856864
}
857865

858-
fn resolve_absolute(path: &str, working_dir: &str) -> String {
866+
pub(crate) fn resolve_absolute(path: &str, working_dir: &str) -> String {
859867
let p = Path::new(path);
860868
let joined = if p.is_absolute() {
861869
p.to_path_buf()

src/ui/mod.rs

Lines changed: 69 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3577,20 +3577,46 @@ pub async fn run_interactive(
35773577
let safe_input = sanitize_output(&ask_req.input);
35783578
// Spacer rows are empty strings — the widget
35793579
// wraps + paints them as a blank row each,
3580-
// giving the alert visual breathing room
3581-
// between sections. Action keys use theme::perm
3582-
// (amber/yellow) so the whole alert reads in
3583-
// one cautionary color. The args line uses an
3584-
// explicit `\x1F` (Unit Separator) sentinel as
3585-
// the hanging-indent boundary so wrapped
3586-
// continuations align under the value, not at
3587-
// column 0. paint_overlay_box reads the
3588-
// sentinel and uses a 6-space wrap indent for
3589-
// that line.
3580+
// effectively adding breathing room above / below
3581+
// the prompt text.
35903582
let mut overlay: Vec<(String, Color)> = Vec::new();
35913583
overlay.push(("⚠ PERMISSION REQUIRED".to_string(), theme::perm()));
35923584
overlay.push((String::new(), theme::perm()));
35933585
overlay.push((format!("tool: {}", safe_tool), theme::perm()));
3586+
3587+
// Show path context for file-operating tools
3588+
// instead of the generic "args:" label.
3589+
let arg_label = match ask_req.tool.as_str() {
3590+
"read" | "write" | "edit" | "list_dir"
3591+
| "apply_patch" | "find_files" | "glob"
3592+
| "list_symbols" | "get_symbol_body"
3593+
| "find_definition" | "find_callers" | "find_callees" => {
3594+
let cwd = session.working_dir.as_str();
3595+
let path_hint = if !cwd.is_empty() {
3596+
let abs = crate::permission::checker::resolve_absolute(
3597+
&ask_req.input, cwd,
3598+
);
3599+
if abs.starts_with(cwd) {
3600+
" (inside project)"
3601+
} else {
3602+
" (outside project)"
3603+
}
3604+
} else {
3605+
""
3606+
};
3607+
format!("path: {}{}", safe_input, path_hint)
3608+
}
3609+
"bash" => format!("command: {}", safe_input),
3610+
"task" | "task_status" => format!("task: {}", safe_input),
3611+
"webfetch" | "websearch" => format!("url: {}", safe_input),
3612+
_ if ask_req.tool.starts_with("mcp_tool") => {
3613+
format!("mcp: {}", safe_input)
3614+
}
3615+
_ => format!("args: {}", safe_input),
3616+
};
3617+
overlay.push((arg_label, theme::perm()));
3618+
overlay.push((String::new(), theme::perm()));
3619+
overlay.push((format!("tool: {}", safe_tool), theme::perm()));
35943620
overlay.push((format!("args: {}", safe_input), theme::perm()));
35953621
overlay.push((String::new(), theme::perm()));
35963622
overlay.push((
@@ -4753,8 +4779,39 @@ fn suggest_pattern(tool: &str, input: &str) -> String {
47534779
PLACEHOLDER.to_string()
47544780
}
47554781
}
4756-
// Other unknown tools (semantic, plugin) — return
4757-
// placeholder so the user explicitly edits before allowing.
4782+
// Other path-shaped tools: derive parent-dir wildcards.
4783+
"apply_patch" => {
4784+
let path = std::path::Path::new(trimmed);
4785+
let parent = path
4786+
.parent()
4787+
.map(|p| p.to_string_lossy())
4788+
.unwrap_or(std::borrow::Cow::Borrowed(""));
4789+
if parent.is_empty() {
4790+
"**".to_string()
4791+
} else {
4792+
format!("{}/**", parent)
4793+
}
4794+
}
4795+
// Semantic tools — file-path-shaped arguments.
4796+
"list_symbols" | "get_symbol_body" | "find_definition" | "find_callers"
4797+
| "find_callees" => {
4798+
let path = std::path::Path::new(trimmed);
4799+
let parent = path
4800+
.parent()
4801+
.map(|p| p.to_string_lossy())
4802+
.unwrap_or(std::borrow::Cow::Borrowed("."));
4803+
format!("{}/**", parent)
4804+
}
4805+
// Network tools — broad wildcard.
4806+
"webfetch" => "webfetch:*".to_string(),
4807+
"websearch" => "websearch:*".to_string(),
4808+
// Task / question / introspection tools — allow them entirely.
4809+
"task" | "task_status" | "question" => "**".to_string(),
4810+
"glob" | "repo_overview" | "skill" | "memory" | "write_todo_list" | "lsp" => {
4811+
"**".to_string()
4812+
}
4813+
// Plugin tools — allow by tool name prefix, default to **.
4814+
_ if tool.starts_with("mcp_tool") => PLACEHOLDER.to_string(),
47584815
_ => PLACEHOLDER.to_string(),
47594816
}
47604817
}

0 commit comments

Comments
 (0)