diff --git a/src-tauri/src/assistant/engine.rs b/src-tauri/src/assistant/engine.rs index 4b3bc81..3c82ec3 100644 --- a/src-tauri/src/assistant/engine.rs +++ b/src-tauri/src/assistant/engine.rs @@ -1115,17 +1115,19 @@ pub(crate) fn build_system_prompt( )); } - if context.execution.shell.allowed_command_prefixes.is_empty() { - let hint = match context.execution.shell.mode { - crate::config::ShellAccessMode::Restricted => "none (no commands allowed)", - _ => "any command not blocked", - }; - prompt.push_str(&format!("- Allowed command prefixes: {}\n", hint)); - } else { - prompt.push_str(&format!( - "- Allowed command prefixes: {}\n", - context.execution.shell.allowed_command_prefixes.join(", ") - )); + match context.execution.shell.mode { + crate::config::ShellAccessMode::Restricted => { + let allowed = context.execution.shell.effective_allowed_command_prefixes(); + let allowed_text = if allowed.is_empty() { + "none".to_string() + } else { + allowed.join(", ") + }; + prompt.push_str(&format!("- Allowed command prefixes: {}\n", allowed_text)); + } + _ => { + prompt.push_str("- Allowed command prefixes: any command not blocked\n"); + } } if context.execution.web.enabled { @@ -1475,7 +1477,7 @@ mod tests { fn build_system_prompt_describes_shell_mode_alongside_memory_guidance() { let mut execution = ExecutionCapabilityConfig::default(); execution.shell.mode = ShellAccessMode::Restricted; - execution.shell.allowed_command_prefixes = vec!["rg".to_string(), "git status".to_string()]; + execution.shell.allowed_command_prefixes = vec!["cargo check".to_string()]; let context = SessionContext { agent_workspace_id: Some("agent-123".to_string()), @@ -1490,7 +1492,8 @@ mod tests { }; assert!(text.contains("- Shell mode: restricted")); - assert!(text.contains("- Allowed command prefixes: rg, git status")); + assert!(text.contains("- Allowed command prefixes: cargo check")); + assert!(text.contains("cargo check")); assert!(text.contains("## Agent Memory")); } diff --git a/src-tauri/src/assistant/tools/local.rs b/src-tauri/src/assistant/tools/local.rs index 828989e..94e755a 100644 --- a/src-tauri/src/assistant/tools/local.rs +++ b/src-tauri/src/assistant/tools/local.rs @@ -1130,6 +1130,11 @@ pub(crate) fn evaluate_command_policy( } let mut approvals: Vec = Vec::new(); + let durable_allowed_prefixes = if matches!(execution.shell.mode, ShellAccessMode::Restricted) { + execution.shell.effective_allowed_command_prefixes() + } else { + Vec::new() + }; for segment in &segments { let text = segment.text(); @@ -1184,8 +1189,7 @@ pub(crate) fn evaluate_command_policy( // surfaces an "Always allow" button for Opaque rows the // same way it does for Simple ones. The user opts into // the broader trust explicitly. - let durable_match = - find_matching_prefix(&execution.shell.allowed_command_prefixes, text); + let durable_match = find_matching_prefix(&durable_allowed_prefixes, text); if durable_match.is_some() || run_match.is_some() { continue; } @@ -1196,8 +1200,7 @@ pub(crate) fn evaluate_command_policy( }); } Segment::Simple(_) => { - let durable_match = - find_matching_prefix(&execution.shell.allowed_command_prefixes, text); + let durable_match = find_matching_prefix(&durable_allowed_prefixes, text); if durable_match.is_none() && run_match.is_none() { approvals.push(SegmentApproval { text: text.to_string(), @@ -3132,10 +3135,23 @@ mod tests { } #[test] - fn policy_restricted_with_no_allow_anywhere_denies() { + fn policy_allows_seeded_restricted_prefix_from_allowlist() { + let exec = restricted_execution_config(&["rg"], &[]); + assert!(enforce_command_policy(&exec, None, "rg --files").is_ok()); + } + + #[test] + fn policy_restricted_prefix_removed_from_allowlist_needs_approval() { + let exec = restricted_execution_config(&[], &[]); + let err = enforce_command_policy(&exec, None, "rg --files").unwrap_err(); + assert!(matches!(err, CommandDenial::NotInAllowList(_))); + } + + #[test] + fn policy_restricted_with_no_matching_allowed_prefix_denies() { let temp = tempdir().unwrap(); let exec = restricted_execution_config(&[], &[]); - let err = enforce_command_policy(&exec, Some(temp.path()), "git status").unwrap_err(); + let err = enforce_command_policy(&exec, Some(temp.path()), "obscure-tool").unwrap_err(); assert!(matches!(err, CommandDenial::NotInAllowList(_))); } @@ -3294,7 +3310,7 @@ mod tests { let exec = restricted_execution_config(&[], &[]); let run_allowed = vec!["git status".to_string()]; assert!(matches!( - evaluate_command_policy(&exec, "git log", &run_allowed, &[]), + evaluate_command_policy(&exec, "git add -A", &run_allowed, &[]), PolicyResult::NeedsApproval(_), )); } @@ -3402,16 +3418,21 @@ mod tests { #[test] fn policy_dedups_repeated_prefix_into_one_approval() { - // The motivating bug: `cd /a && cd /b && cd /c` derives the same - // `cd` prefix three times. The user should be asked to grant `cd` + // The motivating bug: repeated segments can derive the same + // smart prefix multiple times. The user should be asked once, // exactly once, not once per occurrence. let exec = restricted_execution_config(&[], &[]); - let result = evaluate_command_policy(&exec, "cd /a && cd /b && cd /c", &[], &[]); + let result = + evaluate_command_policy(&exec, "git add a && git add b && git add c", &[], &[]); let PolicyResult::NeedsApproval(approvals) = result else { panic!("expected NeedsApproval, got {:?}", policy_label(&result)); }; - assert_eq!(approvals.len(), 1, "repeated `cd` prefix should collapse"); - assert_eq!(approvals[0].suggested_prefix, "cd"); + assert_eq!( + approvals.len(), + 1, + "repeated `git add` prefix should collapse" + ); + assert_eq!(approvals[0].suggested_prefix, "git add"); } #[test] diff --git a/src-tauri/src/commands/permissions.rs b/src-tauri/src/commands/permissions.rs index 7227662..f3fc1df 100644 --- a/src-tauri/src/commands/permissions.rs +++ b/src-tauri/src/commands/permissions.rs @@ -464,13 +464,6 @@ fn apply_decisions_to_shell_policy( if prefix.is_empty() { continue; } - let before = agent.execution.shell.blocked_command_prefixes.len(); - agent - .execution - .shell - .blocked_command_prefixes - .retain(|p| p != prefix); - changed |= agent.execution.shell.blocked_command_prefixes.len() != before; if !agent .execution .shell @@ -491,13 +484,6 @@ fn apply_decisions_to_shell_policy( if prefix.is_empty() { continue; } - let before = agent.execution.shell.allowed_command_prefixes.len(); - agent - .execution - .shell - .allowed_command_prefixes - .retain(|p| p != prefix); - changed |= agent.execution.shell.allowed_command_prefixes.len() != before; if !agent .execution .shell @@ -566,7 +552,7 @@ mod tests { } #[test] - fn allow_always_adds_prefix_and_removes_it_from_blocklist() { + fn allow_always_adds_prefix_without_rewriting_blocklist() { let mut agent = fake_agent(); agent .execution @@ -582,7 +568,7 @@ mod tests { .shell .allowed_command_prefixes .contains(&"cargo".to_string())); - assert!(!agent + assert!(agent .execution .shell .blocked_command_prefixes @@ -590,7 +576,7 @@ mod tests { } #[test] - fn deny_always_adds_to_blocklist_and_removes_from_allowlist() { + fn deny_always_adds_to_blocklist_without_rewriting_allowlist() { let mut agent = fake_agent(); agent .execution @@ -606,7 +592,7 @@ mod tests { .shell .blocked_command_prefixes .contains(&"curl".to_string())); - assert!(!agent + assert!(agent .execution .shell .allowed_command_prefixes @@ -670,14 +656,14 @@ mod tests { assert!(apply_decisions_to_shell_policy( &mut agent, - &[allow_always(" git status ")] + &[allow_always(" cargo check ")] )); assert!(agent .execution .shell .allowed_command_prefixes - .contains(&"git status".to_string())); + .contains(&"cargo check".to_string())); } fn fake_request(workspace_id: Option<&str>) -> PermissionRequest { diff --git a/src-tauri/src/config/types.rs b/src-tauri/src/config/types.rs index d675e72..2db68c3 100644 --- a/src-tauri/src/config/types.rs +++ b/src-tauri/src/config/types.rs @@ -29,6 +29,40 @@ fn default_restricted_shell_blocklist() -> Vec { ] } +/// Built-in command prefixes for Restricted shell mode. These are inspection +/// defaults, not a guarantee that every flag combination is non-mutating; the +/// filesystem sandbox and blocklist still define the hard safety boundary. +pub fn standard_restricted_shell_allowlist() -> Vec { + vec![ + "pwd".to_string(), + "cd".to_string(), + "ls".to_string(), + "rg".to_string(), + "grep".to_string(), + "head".to_string(), + "tail".to_string(), + "wc".to_string(), + "file".to_string(), + "stat".to_string(), + "du".to_string(), + "df".to_string(), + "date".to_string(), + "whoami".to_string(), + "uname".to_string(), + "which".to_string(), + "git status".to_string(), + "git diff".to_string(), + "git log".to_string(), + "git show".to_string(), + "git rev-parse".to_string(), + "git ls-files".to_string(), + "git grep".to_string(), + "git blame".to_string(), + "git branch --show-current".to_string(), + "git remote -v".to_string(), + ] +} + // ============================================================================= // AI Provider // ============================================================================= @@ -299,6 +333,25 @@ impl Default for ShellCapabilityConfig { } } +impl ShellCapabilityConfig { + pub fn effective_allowed_command_prefixes(&self) -> Vec { + let mut allowed = Vec::new(); + for prefix in &self.allowed_command_prefixes { + let prefix = prefix.trim(); + if !prefix.is_empty() { + push_unique_prefix(&mut allowed, prefix.to_string()); + } + } + allowed + } +} + +fn push_unique_prefix(prefixes: &mut Vec, prefix: String) { + if !prefixes.iter().any(|existing| existing == &prefix) { + prefixes.push(prefix); + } +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Default)] #[serde(rename_all = "camelCase")] pub struct WebCapabilityConfig { diff --git a/src-tauri/src/config/workspace_config.rs b/src-tauri/src/config/workspace_config.rs index 679ac28..a96bebf 100644 --- a/src-tauri/src/config/workspace_config.rs +++ b/src-tauri/src/config/workspace_config.rs @@ -5,6 +5,7 @@ use std::path::{Path, PathBuf}; use serde::{Deserialize, Serialize}; use ts_rs::TS; +use crate::config::types::standard_restricted_shell_allowlist; use crate::config::{bundled, AppConfig, SkillSourceKind}; use crate::config::{ ExecutionCapabilityConfig, FilesystemPathAccess, FilesystemPathGrant, ShellAccessMode, @@ -265,6 +266,7 @@ pub fn default_agent_execution() -> ExecutionCapabilityConfig { origin: None, }); } + execution.shell.allowed_command_prefixes = standard_restricted_shell_allowlist(); execution } @@ -569,6 +571,11 @@ mod attach_provider_tests { let manager = WorkspaceAgent::new_manager("mgr".to_string(), 1); assert_eq!(manager.execution.shell.mode, ShellAccessMode::Restricted); assert!(manager.execution.web.enabled); + assert!(manager + .execution + .shell + .allowed_command_prefixes + .contains(&"rg".to_string())); } // ------------------------------------------------------------------- diff --git a/src/components/Settings/WorkspaceSettingsModal.module.css b/src/components/Settings/WorkspaceSettingsModal.module.css index da3af46..51e88f0 100644 --- a/src/components/Settings/WorkspaceSettingsModal.module.css +++ b/src/components/Settings/WorkspaceSettingsModal.module.css @@ -519,6 +519,39 @@ opacity: 1; } +.commandList { + display: flex; + flex-direction: column; + gap: 6px; + max-height: 240px; + overflow-y: auto; + margin-bottom: 8px; + border: 1px solid var(--color-border-light); + border-radius: 6px; + padding: 6px; + background: var(--color-bg-primary); +} + +.commandItem { + display: flex; + align-items: center; + gap: 8px; + min-width: 0; + padding: 6px 8px; + background: var(--color-bg-secondary); + border-radius: 5px; + font-size: 12px; +} + +.commandPrefix { + flex: 1; + min-width: 0; + overflow-wrap: anywhere; + font-family: ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, monospace; + font-size: 12px; + color: var(--color-text-primary); +} + .listInputRow { display: flex; gap: 8px; diff --git a/src/components/Settings/WorkspaceSettingsModal.tsx b/src/components/Settings/WorkspaceSettingsModal.tsx index 7dc4b75..20142b5 100644 --- a/src/components/Settings/WorkspaceSettingsModal.tsx +++ b/src/components/Settings/WorkspaceSettingsModal.tsx @@ -61,7 +61,11 @@ interface PathGrant { interface ExecutionConfig { sandbox: { network: string; sessionBus: string }; filesystem: { extraPaths: PathGrant[] }; - shell: { mode: string; allowedCommandPrefixes: string[]; blockedCommandPrefixes: string[] }; + shell: { + mode: string; + allowedCommandPrefixes: string[]; + blockedCommandPrefixes: string[]; + }; web: { enabled: boolean }; } @@ -171,7 +175,9 @@ const normalizeExecution = (execution: Partial = {}): Execution }, shell: { mode: execution.shell?.mode || d.shell.mode, - allowedCommandPrefixes: normalizeItems(execution.shell?.allowedCommandPrefixes || d.shell.allowedCommandPrefixes), + allowedCommandPrefixes: normalizeItems( + execution.shell?.allowedCommandPrefixes || d.shell.allowedCommandPrefixes + ), blockedCommandPrefixes: normalizeItems(execution.shell?.blockedCommandPrefixes || d.shell.blockedCommandPrefixes), }, web: { enabled: execution.web?.enabled || false }, @@ -1494,14 +1500,30 @@ const AgentSection = ({ setName(selectedTemplate.name || ''); setDescription(selectedTemplate.description || ''); setSelectedSkillIds(selectedTemplate.defaultSkillIds || []); - const execution = normalizeExecution(selectedTemplate.defaultExecution || defaultExecution()); + const execution = normalizeExecution( + selectedTemplate.defaultExecution || deps?.defaultExecution || defaultExecution() + ); setExtraPathGrants(execution.filesystem.extraPaths); setSessionBusAllowed(execution.sandbox.sessionBus === 'allow'); setShellMode(execution.shell.mode); setAllowedCommands(execution.shell.allowedCommandPrefixes); setBlockedCommands(execution.shell.blockedCommandPrefixes); setWebEnabled(execution.web.enabled); - }, [selectedTemplate]); + }, [selectedTemplate, deps?.defaultExecution]); + + const handleAddAllowedCommand = () => { + const prefix = allowedCommandDraft.trim(); + if (!prefix) return; + setAllowedCommands((s) => addUniqueItem(s, prefix)); + setAllowedCommandDraft(''); + }; + + const handleAddBlockedCommand = () => { + const prefix = blockedCommandDraft.trim(); + if (!prefix) return; + setBlockedCommands((s) => addUniqueItem(s, prefix)); + setBlockedCommandDraft(''); + }; const handleAddPathGrant = () => { const path = extraPathDraft.trim(); @@ -1880,10 +1902,10 @@ const AgentSection = ({
{allowedCommands.length > 0 && ( -
+
{allowedCommands.map((cmd) => ( - - {cmd} +
+ {cmd} - +
))}
)} + {allowedCommands.length === 0 && ( + No allowed prefixes configured. + )}
{ if (e.key === 'Enter') { e.preventDefault(); - setAllowedCommands((s) => addUniqueItem(s, allowedCommandDraft)); - setAllowedCommandDraft(''); + handleAddAllowedCommand(); } }} /> @@ -1930,10 +1951,10 @@ const AgentSection = ({
{blockedCommands.length > 0 && ( -
+
{blockedCommands.map((cmd) => ( - - {cmd} +
+ {cmd} - +
))}
)} @@ -1958,19 +1979,15 @@ const AgentSection = ({ onKeyDown={(e) => { if (e.key === 'Enter') { e.preventDefault(); - setBlockedCommands((s) => addUniqueItem(s, blockedCommandDraft)); - setBlockedCommandDraft(''); + handleAddBlockedCommand(); } }} />