Skip to content
Merged
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
65 changes: 64 additions & 1 deletion src/permission/checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,14 +286,30 @@ impl PermissionChecker {
}
}

#[allow(dead_code)]
pub fn allowlist_entries(&self) -> Vec<(String, String)> {
self.session_allowlist
.iter()
.map(|(t, p)| (t.clone(), p.original.clone()))
.collect()
}

/// Remove the allowlist entry at the given index (0-based,
/// matching the display order in `/allow list`). Returns the
/// removed `(tool, pattern)` on success, or `None` if the
/// index is out of range. Used by `/allow remove <n>`.
pub fn remove_session_allowlist_at(&mut self, idx: usize) -> Option<(String, String)> {
if idx >= self.session_allowlist.len() {
return None;
}
let (tool, pat) = self.session_allowlist.remove(idx);
Some((tool, pat.original.clone()))
}

/// Remove ALL allowlist entries. Used by `/allow clear`.
pub fn clear_session_allowlist(&mut self) {
self.session_allowlist.clear();
}

pub fn set_mode(&mut self, mode: SecurityMode) {
self.mode = mode;
}
Expand Down Expand Up @@ -397,6 +413,53 @@ mod tests {
assert!(matches!(r2, CheckResult::Allowed));
}

/// Phase 5 — `/allow remove <idx>` plumbs through to
/// `remove_session_allowlist_at`. Returns the removed entry's
/// (tool, pattern) so the slash handler can confirm to the
/// user what was removed.
#[test]
fn remove_session_allowlist_at_returns_removed_entry() {
let mut checker = fresh_checker();
checker.add_session_allowlist("bash".to_string(), "cargo *");
checker.add_session_allowlist("bash".to_string(), "git *");
checker.add_session_allowlist("read".to_string(), "/tmp/*");
assert_eq!(checker.allowlist_entries().len(), 3);

let removed = checker.remove_session_allowlist_at(1);
assert_eq!(removed, Some(("bash".to_string(), "git *".to_string())),);
// After removal, the indices shift: original [0]bash:cargo*,
// [2]read:/tmp/* are now at [0] and [1].
let after = checker.allowlist_entries();
assert_eq!(after.len(), 2);
assert_eq!(after[0], ("bash".to_string(), "cargo *".to_string()));
assert_eq!(after[1], ("read".to_string(), "/tmp/*".to_string()));
}

/// Out-of-range index returns None rather than panicking. The
/// slash handler shows a clear error in that case.
#[test]
fn remove_session_allowlist_at_out_of_range_returns_none() {
let mut checker = fresh_checker();
checker.add_session_allowlist("bash".to_string(), "cargo *");
assert_eq!(checker.remove_session_allowlist_at(99), None);
assert_eq!(checker.remove_session_allowlist_at(1), None);
// Existing entry still there.
assert_eq!(checker.allowlist_entries().len(), 1);
}

/// `clear` empties the allowlist entirely. Different from
/// `reset_to_new` (which clears EVERYTHING) — this is the
/// user-facing nuke for just allowlist grants.
#[test]
fn clear_session_allowlist_empties_the_list() {
let mut checker = fresh_checker();
checker.add_session_allowlist("bash".to_string(), "cargo *");
checker.add_session_allowlist("bash".to_string(), "git *");
assert_eq!(checker.allowlist_entries().len(), 2);
checker.clear_session_allowlist();
assert!(checker.allowlist_entries().is_empty());
}

// Adding the same (tool, pattern) twice must not duplicate the
// entry. The audit flagged that "allow always" picks for the
// same command repeated across a long session accumulate
Expand Down
153 changes: 153 additions & 0 deletions src/ui/slash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1149,6 +1149,143 @@ pub async fn handle_slash(
}
}
}
"/allow" => {
// Phase 5: CRUD for the session permission allowlist.
// Subcommands: list (default), add <tool> <pattern>,
// remove <idx>, clear. Without this, users can only
// create allowlist entries via the interactive "(a)
// allow always" prompt; there's no way to inspect or
// undo a bad grant short of editing the session JSON.
let sub = parts.get(1).copied().unwrap_or("list");
let perm = match permission {
Some(p) => p,
None => {
renderer.write_line(
"permission system unavailable (--no-tools mode?)",
c_error(),
)?;
return Ok(());
}
};
match sub {
"list" => {
let entries = {
let guard = perm.lock().unwrap_or_else(|e| e.into_inner());
guard.allowlist_entries()
};
if entries.is_empty() {
renderer.write_line(
"session allowlist is empty (use '(a) allow always' in a permission prompt to add entries)",
c_agent(),
)?;
} else {
renderer.write_line(
&format!("session allowlist ({} entries):", entries.len()),
c_agent(),
)?;
for (i, (tool, pat)) in entries.iter().enumerate() {
renderer
.write_line(&format!(" [{}] {} {}", i, tool, pat), c_result())?;
}
renderer.write_line(
"use '/allow remove <idx>' to drop a single entry; '/allow clear' to drop all",
theme::dim(),
)?;
}
}
"add" => {
// `/allow add <tool> <pattern>` — third part is
// the tool, rest is the pattern (may contain
// spaces, so re-derive from raw text).
let raw_args = text.trim().strip_prefix("/allow").unwrap_or("").trim();
let rest = raw_args.strip_prefix("add").unwrap_or("").trim();
let mut it = rest.splitn(2, char::is_whitespace);
let tool = it.next().unwrap_or("");
let pattern = it.next().unwrap_or("").trim();
if tool.is_empty() || pattern.is_empty() {
renderer.write_line(
"usage: /allow add <tool> <pattern> (e.g. /allow add bash 'cargo *')",
c_error(),
)?;
} else {
{
let mut guard = perm.lock().unwrap_or_else(|e| e.into_inner());
guard.add_session_allowlist(tool.to_string(), pattern);
}
// Mirror into session.permission_allowlist
// so the entry persists across save/load.
let entry = crate::session::PermissionAllowEntry {
tool: tool.to_string(),
pattern: pattern.to_string(),
};
// Dedup at the session level too — checker
// dedupes but we want save() to write a
// clean list.
if !session
.permission_allowlist
.iter()
.any(|e| e.tool == entry.tool && e.pattern == entry.pattern)
{
session.permission_allowlist.push(entry);
}
renderer.write_line(&format!("added: {} {}", tool, pattern), c_agent())?;
}
}
"remove" => {
let idx_str = parts.get(2).copied().unwrap_or("");
let idx: usize = match idx_str.parse() {
Ok(n) => n,
Err(_) => {
renderer.write_line(
"usage: /allow remove <idx> (run /allow list to see indices)",
c_error(),
)?;
return Ok(());
}
};
let removed = {
let mut guard = perm.lock().unwrap_or_else(|e| e.into_inner());
guard.remove_session_allowlist_at(idx)
};
match removed {
Some((tool, pat)) => {
// Mirror removal into the session
// allowlist too.
session
.permission_allowlist
.retain(|e| !(e.tool == tool && e.pattern == pat));
renderer.write_line(
&format!("removed [{}]: {} {}", idx, tool, pat),
c_agent(),
)?;
}
None => {
renderer.write_line(
&format!("no allowlist entry at index {}", idx),
c_error(),
)?;
}
}
}
"clear" => {
{
let mut guard = perm.lock().unwrap_or_else(|e| e.into_inner());
guard.clear_session_allowlist();
}
session.permission_allowlist.clear();
renderer.write_line("session allowlist cleared", c_agent())?;
}
other => {
renderer.write_line(
&format!(
"unknown /allow subcommand {:?}; try: list, add, remove, clear",
other,
),
c_error(),
)?;
}
}
}
"/help" => {
renderer.write_line("commands:", c_agent())?;
renderer.write_line(" /model [name] show or switch model", c_result())?;
Expand Down Expand Up @@ -1223,6 +1360,22 @@ pub async fn handle_slash(
" /toggle <feat> [on|off] toggle a feature (e.g. /toggle todo)",
c_result(),
)?;
renderer.write_line(
" /allow list list session allowlist entries",
c_result(),
)?;
renderer.write_line(
" /allow add <tool> <pat> add an allowlist entry",
c_result(),
)?;
renderer.write_line(
" /allow remove <idx> drop one allowlist entry",
c_result(),
)?;
renderer.write_line(
" /allow clear drop all allowlist entries",
c_result(),
)?;
#[cfg(feature = "loop")]
{
let _ = renderer.write_line(
Expand Down
Loading