From 1a02fb7c13ec812ac987972fa4c1d703369d8429 Mon Sep 17 00:00:00 2001 From: juan <2930882+juacker@users.noreply.github.com> Date: Mon, 3 Aug 2026 14:24:28 +0200 Subject: [PATCH 1/2] feat: add standard restricted shell defaults --- src-tauri/src/assistant/engine.rs | 30 ++-- src-tauri/src/assistant/tools/local.rs | 58 +++++-- src-tauri/src/commands/permissions.rs | 67 ++++++-- src-tauri/src/config/types.rs | 73 +++++++++ .../WorkspaceSettingsModal.module.css | 102 ++++++++++++ .../Settings/WorkspaceSettingsModal.tsx | 150 ++++++++++++++++-- 6 files changed, 432 insertions(+), 48 deletions(-) diff --git a/src-tauri/src/assistant/engine.rs b/src-tauri/src/assistant/engine.rs index 4b3bc81..355564c 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 (all restricted defaults disabled and no custom prefixes)".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,9 @@ mod tests { }; assert!(text.contains("- Shell mode: restricted")); - assert!(text.contains("- Allowed command prefixes: rg, git status")); + assert!(text.contains("- Allowed command prefixes: pwd, cd, ls, rg")); + assert!(text.contains("git status")); + 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..4cb2571 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(), @@ -3098,11 +3101,23 @@ mod tests { fn restricted_execution_config( allowed: &[&str], blocked: &[&str], + ) -> ExecutionCapabilityConfig { + restricted_execution_config_with_disabled_defaults(allowed, blocked, &[]) + } + + fn restricted_execution_config_with_disabled_defaults( + allowed: &[&str], + blocked: &[&str], + disabled_defaults: &[&str], ) -> ExecutionCapabilityConfig { ExecutionCapabilityConfig { shell: ShellCapabilityConfig { mode: ShellAccessMode::Restricted, allowed_command_prefixes: allowed.iter().map(|s| s.to_string()).collect(), + disabled_default_command_prefixes: disabled_defaults + .iter() + .map(|s| s.to_string()) + .collect(), blocked_command_prefixes: blocked.iter().map(|s| s.to_string()).collect(), }, ..Default::default() @@ -3132,10 +3147,23 @@ mod tests { } #[test] - fn policy_restricted_with_no_allow_anywhere_denies() { + fn policy_allows_standard_restricted_prefix_without_custom_allowlist() { + let exec = restricted_execution_config(&[], &[]); + assert!(enforce_command_policy(&exec, None, "rg --files").is_ok()); + } + + #[test] + fn policy_disabled_standard_restricted_prefix_needs_approval() { + let exec = restricted_execution_config_with_disabled_defaults(&[], &[], &["rg"]); + 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(_))); } @@ -3211,6 +3239,7 @@ mod tests { shell: ShellCapabilityConfig { mode: ShellAccessMode::Full, allowed_command_prefixes: vec![], + disabled_default_command_prefixes: vec![], blocked_command_prefixes: vec![], }, ..Default::default() @@ -3294,7 +3323,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 +3431,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..ff02de8 100644 --- a/src-tauri/src/commands/permissions.rs +++ b/src-tauri/src/commands/permissions.rs @@ -471,19 +471,40 @@ fn apply_decisions_to_shell_policy( .blocked_command_prefixes .retain(|p| p != prefix); changed |= agent.execution.shell.blocked_command_prefixes.len() != before; - if !agent - .execution - .shell - .allowed_command_prefixes - .iter() - .any(|p| p == prefix) - { + if crate::config::types::ShellCapabilityConfig::is_standard_restricted_prefix( + prefix, + ) { + let before = agent + .execution + .shell + .disabled_default_command_prefixes + .len(); agent + .execution + .shell + .disabled_default_command_prefixes + .retain(|p| p != prefix); + changed |= agent + .execution + .shell + .disabled_default_command_prefixes + .len() + != before; + } else { + if !agent .execution .shell .allowed_command_prefixes - .push(prefix.to_string()); - changed = true; + .iter() + .any(|p| p == prefix) + { + agent + .execution + .shell + .allowed_command_prefixes + .push(prefix.to_string()); + changed = true; + } } } SegmentDecision::DenyAlways { prefix, .. } => { @@ -670,14 +691,38 @@ 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())); + } + + #[test] + fn allow_always_for_standard_prefix_reenables_default_without_custom_chip() { + let mut agent = fake_agent(); + agent + .execution + .shell + .disabled_default_command_prefixes + .push("rg".to_string()); + + let changed = apply_decisions_to_shell_policy(&mut agent, &[allow_always("rg")]); + + assert!(changed); + assert!(!agent + .execution + .shell + .disabled_default_command_prefixes + .contains(&"rg".to_string())); + assert!(!agent + .execution + .shell + .allowed_command_prefixes + .contains(&"rg".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..cf695a0 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 // ============================================================================= @@ -285,6 +319,8 @@ pub struct ShellCapabilityConfig { pub mode: ShellAccessMode, #[serde(default)] pub allowed_command_prefixes: Vec, + #[serde(default)] + pub disabled_default_command_prefixes: Vec, #[serde(default = "default_restricted_shell_blocklist")] pub blocked_command_prefixes: Vec, } @@ -294,11 +330,48 @@ impl Default for ShellCapabilityConfig { Self { mode: ShellAccessMode::Off, allowed_command_prefixes: Vec::new(), + disabled_default_command_prefixes: Vec::new(), blocked_command_prefixes: default_restricted_shell_blocklist(), } } } +impl ShellCapabilityConfig { + pub fn effective_allowed_command_prefixes(&self) -> Vec { + let mut allowed = Vec::new(); + for prefix in standard_restricted_shell_allowlist() { + if !self + .disabled_default_command_prefixes + .iter() + .any(|disabled| disabled.trim() == prefix) + { + push_unique_prefix(&mut allowed, prefix); + } + } + for prefix in &self.allowed_command_prefixes { + let prefix = prefix.trim(); + if !prefix.is_empty() { + push_unique_prefix(&mut allowed, prefix.to_string()); + } + } + allowed + } + + pub fn is_standard_restricted_prefix(prefix: &str) -> bool { + let prefix = prefix.trim(); + !prefix.is_empty() + && standard_restricted_shell_allowlist() + .iter() + .any(|default_prefix| default_prefix == prefix) + } +} + +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/components/Settings/WorkspaceSettingsModal.module.css b/src/components/Settings/WorkspaceSettingsModal.module.css index da3af46..6eaa7f2 100644 --- a/src/components/Settings/WorkspaceSettingsModal.module.css +++ b/src/components/Settings/WorkspaceSettingsModal.module.css @@ -519,6 +519,108 @@ opacity: 1; } +.permissionHeader { + display: flex; + align-items: center; + justify-content: space-between; + gap: 12px; + margin-bottom: 6px; +} + +.permissionHeader .label { + margin-bottom: 0; +} + +.permissionCount { + font-size: 11px; + color: var(--color-text-tertiary); + white-space: nowrap; +} + +.commandGroupList { + border: 1px solid var(--color-border-light); + border-radius: 6px; + overflow: hidden; + background: var(--color-bg-primary); +} + +.commandGroup + .commandGroup { + border-top: 1px solid var(--color-border-light); +} + +.commandGroupSummary { + display: flex; + align-items: center; + justify-content: space-between; + gap: 12px; + padding: 8px 10px; + cursor: pointer; + color: var(--color-text-secondary); + font-size: 12px; + font-weight: 600; +} + +.commandGroupSummary:hover { + background: var(--color-bg-hover); +} + +.commandGroupSummary span:last-child { + color: var(--color-text-tertiary); + font-weight: 500; +} + +.defaultCommandGrid { + display: grid; + grid-template-columns: repeat(auto-fit, minmax(170px, 1fr)); + gap: 4px 8px; + padding: 0 10px 10px 10px; +} + +.defaultCommandOption { + display: flex; + align-items: center; + gap: 7px; + min-width: 0; + padding: 3px 0; + font-size: 12px; + color: var(--color-text-secondary); +} + +.defaultCommandOption input { + margin: 0; + flex-shrink: 0; +} + +.defaultCommandOption code { + min-width: 0; + overflow-wrap: anywhere; + font-family: ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, monospace; + font-size: 11px; + color: var(--color-text-primary); +} + +.inlineButton { + appearance: none; + margin-top: 8px; + padding: 0; + border: none; + background: transparent; + color: var(--color-primary); + font: inherit; + font-size: 12px; + font-weight: 500; + cursor: pointer; +} + +.inlineButton:hover:not(:disabled) { + text-decoration: underline; +} + +.inlineButton:disabled { + opacity: 0.5; + cursor: not-allowed; +} + .listInputRow { display: flex; gap: 8px; diff --git a/src/components/Settings/WorkspaceSettingsModal.tsx b/src/components/Settings/WorkspaceSettingsModal.tsx index 7dc4b75..10f7d74 100644 --- a/src/components/Settings/WorkspaceSettingsModal.tsx +++ b/src/components/Settings/WorkspaceSettingsModal.tsx @@ -61,7 +61,12 @@ interface PathGrant { interface ExecutionConfig { sandbox: { network: string; sessionBus: string }; filesystem: { extraPaths: PathGrant[] }; - shell: { mode: string; allowedCommandPrefixes: string[]; blockedCommandPrefixes: string[] }; + shell: { + mode: string; + allowedCommandPrefixes: string[]; + disabledDefaultCommandPrefixes: string[]; + blockedCommandPrefixes: string[]; + }; web: { enabled: boolean }; } @@ -118,6 +123,7 @@ const defaultExecution = (): ExecutionConfig => ({ shell: { mode: 'off', allowedCommandPrefixes: [], + disabledDefaultCommandPrefixes: [], blockedCommandPrefixes: [ 'rm', 'sudo', 'chmod', 'chown', 'dd', 'mkfs', 'mount', 'umount', 'shutdown', 'reboot', ], @@ -125,9 +131,46 @@ const defaultExecution = (): ExecutionConfig => ({ web: { enabled: false }, }); +// Mirrors `standard_restricted_shell_allowlist` in src-tauri/src/config/types.rs. +// Keep the UI grouping in sync with backend policy until this metadata is +// exposed through typed execution bindings. +const STANDARD_RESTRICTED_COMMAND_GROUPS = [ + { + label: 'Workspace inspection', + prefixes: ['pwd', 'cd', 'ls', 'rg', 'grep', 'head', 'tail', 'wc', 'file', 'stat'], + }, + { + label: 'System inspection', + prefixes: ['du', 'df', 'date', 'whoami', 'uname', 'which'], + }, + { + label: 'Git inspection', + prefixes: [ + 'git status', + 'git diff', + 'git log', + 'git show', + 'git rev-parse', + 'git ls-files', + 'git grep', + 'git blame', + 'git branch --show-current', + 'git remote -v', + ], + }, +]; + +const STANDARD_RESTRICTED_COMMAND_PREFIXES = STANDARD_RESTRICTED_COMMAND_GROUPS.flatMap( + (group) => group.prefixes +); +const STANDARD_RESTRICTED_COMMAND_SET = new Set(STANDARD_RESTRICTED_COMMAND_PREFIXES); + const normalizeItems = (items: string[] = []): string[] => items.map((item) => item.trim()).filter(Boolean); +const normalizeCustomAllowedCommands = (items: string[] = []): string[] => + normalizeItems(items).filter((item) => !STANDARD_RESTRICTED_COMMAND_SET.has(item)); + const addUniqueItem = (items: string[], value: string): string[] => { const trimmed = value.trim(); if (!trimmed || items.includes(trimmed)) return items; @@ -171,7 +214,12 @@ const normalizeExecution = (execution: Partial = {}): Execution }, shell: { mode: execution.shell?.mode || d.shell.mode, - allowedCommandPrefixes: normalizeItems(execution.shell?.allowedCommandPrefixes || d.shell.allowedCommandPrefixes), + allowedCommandPrefixes: normalizeCustomAllowedCommands( + execution.shell?.allowedCommandPrefixes || d.shell.allowedCommandPrefixes + ), + disabledDefaultCommandPrefixes: normalizeItems( + execution.shell?.disabledDefaultCommandPrefixes || d.shell.disabledDefaultCommandPrefixes + ).filter((item) => STANDARD_RESTRICTED_COMMAND_SET.has(item)), blockedCommandPrefixes: normalizeItems(execution.shell?.blockedCommandPrefixes || d.shell.blockedCommandPrefixes), }, web: { enabled: execution.web?.enabled || false }, @@ -192,6 +240,7 @@ interface AgentPayloadInput { extraPathGrants?: PathGrant[]; shellMode?: string; allowedCommands?: string[]; + disabledDefaultCommands?: string[]; blockedCommands?: string[]; webEnabled?: boolean; enabled?: boolean; @@ -207,6 +256,7 @@ const serializeAgentPayload = ({ extraPathGrants, shellMode, allowedCommands, + disabledDefaultCommands, blockedCommands, webEnabled, enabled, @@ -222,6 +272,7 @@ const serializeAgentPayload = ({ shell: { mode: shellMode, allowedCommandPrefixes: allowedCommands || [], + disabledDefaultCommandPrefixes: disabledDefaultCommands || [], blockedCommandPrefixes: blockedCommands || [], }, web: { enabled: !!webEnabled }, @@ -1332,6 +1383,7 @@ const AgentSection = ({ const [sessionBusAllowed, setSessionBusAllowed] = useState(true); const [shellMode, setShellMode] = useState('off'); const [allowedCommands, setAllowedCommands] = useState([]); + const [disabledDefaultCommands, setDisabledDefaultCommands] = useState([]); const [blockedCommands, setBlockedCommands] = useState(defaultExecution().shell.blockedCommandPrefixes); const [allowedCommandDraft, setAllowedCommandDraft] = useState(''); const [blockedCommandDraft, setBlockedCommandDraft] = useState(''); @@ -1405,6 +1457,7 @@ const AgentSection = ({ setSessionBusAllowed(execution.sandbox.sessionBus === 'allow'); setShellMode(execution.shell.mode); setAllowedCommands(execution.shell.allowedCommandPrefixes); + setDisabledDefaultCommands(execution.shell.disabledDefaultCommandPrefixes); setBlockedCommands(execution.shell.blockedCommandPrefixes); setAllowedCommandDraft(''); setBlockedCommandDraft(''); @@ -1425,6 +1478,7 @@ const AgentSection = ({ extraPathGrants: execution.filesystem.extraPaths, shellMode: execution.shell.mode, allowedCommands: execution.shell.allowedCommandPrefixes, + disabledDefaultCommands: execution.shell.disabledDefaultCommandPrefixes, blockedCommands: execution.shell.blockedCommandPrefixes, webEnabled: execution.web.enabled, enabled: agent.enabled !== false, @@ -1464,13 +1518,14 @@ const AgentSection = ({ extraPathGrants, shellMode, allowedCommands, + disabledDefaultCommands, blockedCommands, webEnabled, enabled, }), [ name, description, selectedSkillIds, selectedMcpServerIds, providerConnectionIds, - sessionBusAllowed, extraPathGrants, shellMode, allowedCommands, blockedCommands, + sessionBusAllowed, extraPathGrants, shellMode, allowedCommands, disabledDefaultCommands, blockedCommands, webEnabled, enabled, ] ); @@ -1499,10 +1554,37 @@ const AgentSection = ({ setSessionBusAllowed(execution.sandbox.sessionBus === 'allow'); setShellMode(execution.shell.mode); setAllowedCommands(execution.shell.allowedCommandPrefixes); + setDisabledDefaultCommands(execution.shell.disabledDefaultCommandPrefixes); setBlockedCommands(execution.shell.blockedCommandPrefixes); setWebEnabled(execution.web.enabled); }, [selectedTemplate]); + const handleSetDefaultCommandEnabled = (prefix: string, enabledDefault: boolean) => { + setDisabledDefaultCommands((current) => { + if (enabledDefault) return current.filter((item) => item !== prefix); + return addUniqueItem(current, prefix); + }); + }; + + const handleAddAllowedCommand = () => { + const prefix = allowedCommandDraft.trim(); + if (!prefix) return; + if (STANDARD_RESTRICTED_COMMAND_SET.has(prefix)) { + handleSetDefaultCommandEnabled(prefix, true); + } else { + setAllowedCommands((s) => addUniqueItem(s, prefix)); + } + setAllowedCommandDraft(''); + }; + + const disabledDefaultCommandSet = useMemo( + () => new Set(disabledDefaultCommands), + [disabledDefaultCommands] + ); + const enabledDefaultCommandCount = STANDARD_RESTRICTED_COMMAND_PREFIXES.filter( + (cmd) => !disabledDefaultCommandSet.has(cmd) + ).length; + const handleAddPathGrant = () => { const path = extraPathDraft.trim(); if (!path) return; @@ -1564,6 +1646,7 @@ const AgentSection = ({ shell: { mode: shellMode, allowedCommandPrefixes: allowedCommands, + disabledDefaultCommandPrefixes: disabledDefaultCommands, blockedCommandPrefixes: blockedCommands, }, web: { enabled: webEnabled }, @@ -1878,7 +1961,54 @@ const AgentSection = ({ {shellMode === 'restricted' && ( <>
- +
+ + + {enabledDefaultCommandCount}/{STANDARD_RESTRICTED_COMMAND_PREFIXES.length} enabled + +
+
+ {STANDARD_RESTRICTED_COMMAND_GROUPS.map((group) => { + const enabledInGroup = group.prefixes.filter( + (cmd) => !disabledDefaultCommandSet.has(cmd) + ).length; + return ( +
+ + {group.label} + {enabledInGroup}/{group.prefixes.length} + +
+ {group.prefixes.map((cmd) => ( + + ))} +
+
+ ); + })} +
+ {disabledDefaultCommands.length > 0 && ( + + )} +
+ +
+ {allowedCommands.length > 0 && (
{allowedCommands.map((cmd) => ( @@ -1908,19 +2038,15 @@ const AgentSection = ({ onKeyDown={(e) => { if (e.key === 'Enter') { e.preventDefault(); - setAllowedCommands((s) => addUniqueItem(s, allowedCommandDraft)); - setAllowedCommandDraft(''); + handleAddAllowedCommand(); } }} /> @@ -1970,7 +2096,7 @@ const AgentSection = ({ setBlockedCommands((s) => addUniqueItem(s, blockedCommandDraft)); setBlockedCommandDraft(''); }} - disabled={!blockedCommandDraft.trim() || saving} + disabled={!blockedCommandDraft.trim() || busy} > Add From c3118fd252fbea502e21ae7ddb9c44612bde08c8 Mon Sep 17 00:00:00 2001 From: juan <2930882+juacker@users.noreply.github.com> Date: Mon, 3 Aug 2026 16:08:08 +0200 Subject: [PATCH 2/2] fix: flatten restricted shell policy lists --- src-tauri/src/assistant/engine.rs | 5 +- src-tauri/src/assistant/tools/local.rs | 21 +-- src-tauri/src/commands/permissions.rs | 81 ++------- src-tauri/src/config/types.rs | 20 --- src-tauri/src/config/workspace_config.rs | 7 + .../WorkspaceSettingsModal.module.css | 99 ++--------- .../Settings/WorkspaceSettingsModal.tsx | 163 +++--------------- 7 files changed, 66 insertions(+), 330 deletions(-) diff --git a/src-tauri/src/assistant/engine.rs b/src-tauri/src/assistant/engine.rs index 355564c..3c82ec3 100644 --- a/src-tauri/src/assistant/engine.rs +++ b/src-tauri/src/assistant/engine.rs @@ -1119,7 +1119,7 @@ pub(crate) fn build_system_prompt( crate::config::ShellAccessMode::Restricted => { let allowed = context.execution.shell.effective_allowed_command_prefixes(); let allowed_text = if allowed.is_empty() { - "none (all restricted defaults disabled and no custom prefixes)".to_string() + "none".to_string() } else { allowed.join(", ") }; @@ -1492,8 +1492,7 @@ mod tests { }; assert!(text.contains("- Shell mode: restricted")); - assert!(text.contains("- Allowed command prefixes: pwd, cd, ls, rg")); - assert!(text.contains("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 4cb2571..94e755a 100644 --- a/src-tauri/src/assistant/tools/local.rs +++ b/src-tauri/src/assistant/tools/local.rs @@ -3101,23 +3101,11 @@ mod tests { fn restricted_execution_config( allowed: &[&str], blocked: &[&str], - ) -> ExecutionCapabilityConfig { - restricted_execution_config_with_disabled_defaults(allowed, blocked, &[]) - } - - fn restricted_execution_config_with_disabled_defaults( - allowed: &[&str], - blocked: &[&str], - disabled_defaults: &[&str], ) -> ExecutionCapabilityConfig { ExecutionCapabilityConfig { shell: ShellCapabilityConfig { mode: ShellAccessMode::Restricted, allowed_command_prefixes: allowed.iter().map(|s| s.to_string()).collect(), - disabled_default_command_prefixes: disabled_defaults - .iter() - .map(|s| s.to_string()) - .collect(), blocked_command_prefixes: blocked.iter().map(|s| s.to_string()).collect(), }, ..Default::default() @@ -3147,14 +3135,14 @@ mod tests { } #[test] - fn policy_allows_standard_restricted_prefix_without_custom_allowlist() { - let exec = restricted_execution_config(&[], &[]); + 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_disabled_standard_restricted_prefix_needs_approval() { - let exec = restricted_execution_config_with_disabled_defaults(&[], &[], &["rg"]); + 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(_))); } @@ -3239,7 +3227,6 @@ mod tests { shell: ShellCapabilityConfig { mode: ShellAccessMode::Full, allowed_command_prefixes: vec![], - disabled_default_command_prefixes: vec![], blocked_command_prefixes: vec![], }, ..Default::default() diff --git a/src-tauri/src/commands/permissions.rs b/src-tauri/src/commands/permissions.rs index ff02de8..f3fc1df 100644 --- a/src-tauri/src/commands/permissions.rs +++ b/src-tauri/src/commands/permissions.rs @@ -464,47 +464,19 @@ fn apply_decisions_to_shell_policy( if prefix.is_empty() { continue; } - let before = agent.execution.shell.blocked_command_prefixes.len(); - agent + if !agent .execution .shell - .blocked_command_prefixes - .retain(|p| p != prefix); - changed |= agent.execution.shell.blocked_command_prefixes.len() != before; - if crate::config::types::ShellCapabilityConfig::is_standard_restricted_prefix( - prefix, - ) { - let before = agent - .execution - .shell - .disabled_default_command_prefixes - .len(); + .allowed_command_prefixes + .iter() + .any(|p| p == prefix) + { agent - .execution - .shell - .disabled_default_command_prefixes - .retain(|p| p != prefix); - changed |= agent - .execution - .shell - .disabled_default_command_prefixes - .len() - != before; - } else { - if !agent .execution .shell .allowed_command_prefixes - .iter() - .any(|p| p == prefix) - { - agent - .execution - .shell - .allowed_command_prefixes - .push(prefix.to_string()); - changed = true; - } + .push(prefix.to_string()); + changed = true; } } SegmentDecision::DenyAlways { prefix, .. } => { @@ -512,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 @@ -587,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 @@ -603,7 +568,7 @@ mod tests { .shell .allowed_command_prefixes .contains(&"cargo".to_string())); - assert!(!agent + assert!(agent .execution .shell .blocked_command_prefixes @@ -611,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 @@ -627,7 +592,7 @@ mod tests { .shell .blocked_command_prefixes .contains(&"curl".to_string())); - assert!(!agent + assert!(agent .execution .shell .allowed_command_prefixes @@ -701,30 +666,6 @@ mod tests { .contains(&"cargo check".to_string())); } - #[test] - fn allow_always_for_standard_prefix_reenables_default_without_custom_chip() { - let mut agent = fake_agent(); - agent - .execution - .shell - .disabled_default_command_prefixes - .push("rg".to_string()); - - let changed = apply_decisions_to_shell_policy(&mut agent, &[allow_always("rg")]); - - assert!(changed); - assert!(!agent - .execution - .shell - .disabled_default_command_prefixes - .contains(&"rg".to_string())); - assert!(!agent - .execution - .shell - .allowed_command_prefixes - .contains(&"rg".to_string())); - } - fn fake_request(workspace_id: Option<&str>) -> PermissionRequest { PermissionRequest { request_id: uuid::Uuid::new_v4().to_string(), diff --git a/src-tauri/src/config/types.rs b/src-tauri/src/config/types.rs index cf695a0..2db68c3 100644 --- a/src-tauri/src/config/types.rs +++ b/src-tauri/src/config/types.rs @@ -319,8 +319,6 @@ pub struct ShellCapabilityConfig { pub mode: ShellAccessMode, #[serde(default)] pub allowed_command_prefixes: Vec, - #[serde(default)] - pub disabled_default_command_prefixes: Vec, #[serde(default = "default_restricted_shell_blocklist")] pub blocked_command_prefixes: Vec, } @@ -330,7 +328,6 @@ impl Default for ShellCapabilityConfig { Self { mode: ShellAccessMode::Off, allowed_command_prefixes: Vec::new(), - disabled_default_command_prefixes: Vec::new(), blocked_command_prefixes: default_restricted_shell_blocklist(), } } @@ -339,15 +336,6 @@ impl Default for ShellCapabilityConfig { impl ShellCapabilityConfig { pub fn effective_allowed_command_prefixes(&self) -> Vec { let mut allowed = Vec::new(); - for prefix in standard_restricted_shell_allowlist() { - if !self - .disabled_default_command_prefixes - .iter() - .any(|disabled| disabled.trim() == prefix) - { - push_unique_prefix(&mut allowed, prefix); - } - } for prefix in &self.allowed_command_prefixes { let prefix = prefix.trim(); if !prefix.is_empty() { @@ -356,14 +344,6 @@ impl ShellCapabilityConfig { } allowed } - - pub fn is_standard_restricted_prefix(prefix: &str) -> bool { - let prefix = prefix.trim(); - !prefix.is_empty() - && standard_restricted_shell_allowlist() - .iter() - .any(|default_prefix| default_prefix == prefix) - } } fn push_unique_prefix(prefixes: &mut Vec, prefix: String) { 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 6eaa7f2..51e88f0 100644 --- a/src/components/Settings/WorkspaceSettingsModal.module.css +++ b/src/components/Settings/WorkspaceSettingsModal.module.css @@ -519,106 +519,37 @@ opacity: 1; } -.permissionHeader { +.commandList { display: flex; - align-items: center; - justify-content: space-between; - gap: 12px; - margin-bottom: 6px; -} - -.permissionHeader .label { - margin-bottom: 0; -} - -.permissionCount { - font-size: 11px; - color: var(--color-text-tertiary); - white-space: nowrap; -} - -.commandGroupList { + flex-direction: column; + gap: 6px; + max-height: 240px; + overflow-y: auto; + margin-bottom: 8px; border: 1px solid var(--color-border-light); border-radius: 6px; - overflow: hidden; + padding: 6px; background: var(--color-bg-primary); } -.commandGroup + .commandGroup { - border-top: 1px solid var(--color-border-light); -} - -.commandGroupSummary { +.commandItem { display: flex; align-items: center; - justify-content: space-between; - gap: 12px; - padding: 8px 10px; - cursor: pointer; - color: var(--color-text-secondary); - font-size: 12px; - font-weight: 600; -} - -.commandGroupSummary:hover { - background: var(--color-bg-hover); -} - -.commandGroupSummary span:last-child { - color: var(--color-text-tertiary); - font-weight: 500; -} - -.defaultCommandGrid { - display: grid; - grid-template-columns: repeat(auto-fit, minmax(170px, 1fr)); - gap: 4px 8px; - padding: 0 10px 10px 10px; -} - -.defaultCommandOption { - display: flex; - align-items: center; - gap: 7px; + gap: 8px; min-width: 0; - padding: 3px 0; + padding: 6px 8px; + background: var(--color-bg-secondary); + border-radius: 5px; font-size: 12px; - color: var(--color-text-secondary); -} - -.defaultCommandOption input { - margin: 0; - flex-shrink: 0; } -.defaultCommandOption code { +.commandPrefix { + flex: 1; min-width: 0; overflow-wrap: anywhere; font-family: ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, monospace; - font-size: 11px; - color: var(--color-text-primary); -} - -.inlineButton { - appearance: none; - margin-top: 8px; - padding: 0; - border: none; - background: transparent; - color: var(--color-primary); - font: inherit; font-size: 12px; - font-weight: 500; - cursor: pointer; -} - -.inlineButton:hover:not(:disabled) { - text-decoration: underline; -} - -.inlineButton:disabled { - opacity: 0.5; - cursor: not-allowed; + color: var(--color-text-primary); } .listInputRow { diff --git a/src/components/Settings/WorkspaceSettingsModal.tsx b/src/components/Settings/WorkspaceSettingsModal.tsx index 10f7d74..20142b5 100644 --- a/src/components/Settings/WorkspaceSettingsModal.tsx +++ b/src/components/Settings/WorkspaceSettingsModal.tsx @@ -64,7 +64,6 @@ interface ExecutionConfig { shell: { mode: string; allowedCommandPrefixes: string[]; - disabledDefaultCommandPrefixes: string[]; blockedCommandPrefixes: string[]; }; web: { enabled: boolean }; @@ -123,7 +122,6 @@ const defaultExecution = (): ExecutionConfig => ({ shell: { mode: 'off', allowedCommandPrefixes: [], - disabledDefaultCommandPrefixes: [], blockedCommandPrefixes: [ 'rm', 'sudo', 'chmod', 'chown', 'dd', 'mkfs', 'mount', 'umount', 'shutdown', 'reboot', ], @@ -131,46 +129,9 @@ const defaultExecution = (): ExecutionConfig => ({ web: { enabled: false }, }); -// Mirrors `standard_restricted_shell_allowlist` in src-tauri/src/config/types.rs. -// Keep the UI grouping in sync with backend policy until this metadata is -// exposed through typed execution bindings. -const STANDARD_RESTRICTED_COMMAND_GROUPS = [ - { - label: 'Workspace inspection', - prefixes: ['pwd', 'cd', 'ls', 'rg', 'grep', 'head', 'tail', 'wc', 'file', 'stat'], - }, - { - label: 'System inspection', - prefixes: ['du', 'df', 'date', 'whoami', 'uname', 'which'], - }, - { - label: 'Git inspection', - prefixes: [ - 'git status', - 'git diff', - 'git log', - 'git show', - 'git rev-parse', - 'git ls-files', - 'git grep', - 'git blame', - 'git branch --show-current', - 'git remote -v', - ], - }, -]; - -const STANDARD_RESTRICTED_COMMAND_PREFIXES = STANDARD_RESTRICTED_COMMAND_GROUPS.flatMap( - (group) => group.prefixes -); -const STANDARD_RESTRICTED_COMMAND_SET = new Set(STANDARD_RESTRICTED_COMMAND_PREFIXES); - const normalizeItems = (items: string[] = []): string[] => items.map((item) => item.trim()).filter(Boolean); -const normalizeCustomAllowedCommands = (items: string[] = []): string[] => - normalizeItems(items).filter((item) => !STANDARD_RESTRICTED_COMMAND_SET.has(item)); - const addUniqueItem = (items: string[], value: string): string[] => { const trimmed = value.trim(); if (!trimmed || items.includes(trimmed)) return items; @@ -214,12 +175,9 @@ const normalizeExecution = (execution: Partial = {}): Execution }, shell: { mode: execution.shell?.mode || d.shell.mode, - allowedCommandPrefixes: normalizeCustomAllowedCommands( + allowedCommandPrefixes: normalizeItems( execution.shell?.allowedCommandPrefixes || d.shell.allowedCommandPrefixes ), - disabledDefaultCommandPrefixes: normalizeItems( - execution.shell?.disabledDefaultCommandPrefixes || d.shell.disabledDefaultCommandPrefixes - ).filter((item) => STANDARD_RESTRICTED_COMMAND_SET.has(item)), blockedCommandPrefixes: normalizeItems(execution.shell?.blockedCommandPrefixes || d.shell.blockedCommandPrefixes), }, web: { enabled: execution.web?.enabled || false }, @@ -240,7 +198,6 @@ interface AgentPayloadInput { extraPathGrants?: PathGrant[]; shellMode?: string; allowedCommands?: string[]; - disabledDefaultCommands?: string[]; blockedCommands?: string[]; webEnabled?: boolean; enabled?: boolean; @@ -256,7 +213,6 @@ const serializeAgentPayload = ({ extraPathGrants, shellMode, allowedCommands, - disabledDefaultCommands, blockedCommands, webEnabled, enabled, @@ -272,7 +228,6 @@ const serializeAgentPayload = ({ shell: { mode: shellMode, allowedCommandPrefixes: allowedCommands || [], - disabledDefaultCommandPrefixes: disabledDefaultCommands || [], blockedCommandPrefixes: blockedCommands || [], }, web: { enabled: !!webEnabled }, @@ -1383,7 +1338,6 @@ const AgentSection = ({ const [sessionBusAllowed, setSessionBusAllowed] = useState(true); const [shellMode, setShellMode] = useState('off'); const [allowedCommands, setAllowedCommands] = useState([]); - const [disabledDefaultCommands, setDisabledDefaultCommands] = useState([]); const [blockedCommands, setBlockedCommands] = useState(defaultExecution().shell.blockedCommandPrefixes); const [allowedCommandDraft, setAllowedCommandDraft] = useState(''); const [blockedCommandDraft, setBlockedCommandDraft] = useState(''); @@ -1457,7 +1411,6 @@ const AgentSection = ({ setSessionBusAllowed(execution.sandbox.sessionBus === 'allow'); setShellMode(execution.shell.mode); setAllowedCommands(execution.shell.allowedCommandPrefixes); - setDisabledDefaultCommands(execution.shell.disabledDefaultCommandPrefixes); setBlockedCommands(execution.shell.blockedCommandPrefixes); setAllowedCommandDraft(''); setBlockedCommandDraft(''); @@ -1478,7 +1431,6 @@ const AgentSection = ({ extraPathGrants: execution.filesystem.extraPaths, shellMode: execution.shell.mode, allowedCommands: execution.shell.allowedCommandPrefixes, - disabledDefaultCommands: execution.shell.disabledDefaultCommandPrefixes, blockedCommands: execution.shell.blockedCommandPrefixes, webEnabled: execution.web.enabled, enabled: agent.enabled !== false, @@ -1518,14 +1470,13 @@ const AgentSection = ({ extraPathGrants, shellMode, allowedCommands, - disabledDefaultCommands, blockedCommands, webEnabled, enabled, }), [ name, description, selectedSkillIds, selectedMcpServerIds, providerConnectionIds, - sessionBusAllowed, extraPathGrants, shellMode, allowedCommands, disabledDefaultCommands, blockedCommands, + sessionBusAllowed, extraPathGrants, shellMode, allowedCommands, blockedCommands, webEnabled, enabled, ] ); @@ -1549,41 +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); - setDisabledDefaultCommands(execution.shell.disabledDefaultCommandPrefixes); setBlockedCommands(execution.shell.blockedCommandPrefixes); setWebEnabled(execution.web.enabled); - }, [selectedTemplate]); - - const handleSetDefaultCommandEnabled = (prefix: string, enabledDefault: boolean) => { - setDisabledDefaultCommands((current) => { - if (enabledDefault) return current.filter((item) => item !== prefix); - return addUniqueItem(current, prefix); - }); - }; + }, [selectedTemplate, deps?.defaultExecution]); const handleAddAllowedCommand = () => { const prefix = allowedCommandDraft.trim(); if (!prefix) return; - if (STANDARD_RESTRICTED_COMMAND_SET.has(prefix)) { - handleSetDefaultCommandEnabled(prefix, true); - } else { - setAllowedCommands((s) => addUniqueItem(s, prefix)); - } + setAllowedCommands((s) => addUniqueItem(s, prefix)); setAllowedCommandDraft(''); }; - const disabledDefaultCommandSet = useMemo( - () => new Set(disabledDefaultCommands), - [disabledDefaultCommands] - ); - const enabledDefaultCommandCount = STANDARD_RESTRICTED_COMMAND_PREFIXES.filter( - (cmd) => !disabledDefaultCommandSet.has(cmd) - ).length; + const handleAddBlockedCommand = () => { + const prefix = blockedCommandDraft.trim(); + if (!prefix) return; + setBlockedCommands((s) => addUniqueItem(s, prefix)); + setBlockedCommandDraft(''); + }; const handleAddPathGrant = () => { const path = extraPathDraft.trim(); @@ -1646,7 +1586,6 @@ const AgentSection = ({ shell: { mode: shellMode, allowedCommandPrefixes: allowedCommands, - disabledDefaultCommandPrefixes: disabledDefaultCommands, blockedCommandPrefixes: blockedCommands, }, web: { enabled: webEnabled }, @@ -1961,59 +1900,12 @@ const AgentSection = ({ {shellMode === 'restricted' && ( <>
-
- - - {enabledDefaultCommandCount}/{STANDARD_RESTRICTED_COMMAND_PREFIXES.length} enabled - -
-
- {STANDARD_RESTRICTED_COMMAND_GROUPS.map((group) => { - const enabledInGroup = group.prefixes.filter( - (cmd) => !disabledDefaultCommandSet.has(cmd) - ).length; - return ( -
- - {group.label} - {enabledInGroup}/{group.prefixes.length} - -
- {group.prefixes.map((cmd) => ( - - ))} -
-
- ); - })} -
- {disabledDefaultCommands.length > 0 && ( - - )} -
- -
- + {allowedCommands.length > 0 && ( -
+
{allowedCommands.map((cmd) => ( - - {cmd} +
+ {cmd} - +
))}
)} + {allowedCommands.length === 0 && ( + No allowed prefixes configured. + )}
{blockedCommands.length > 0 && ( -
+
{blockedCommands.map((cmd) => ( - - {cmd} +
+ {cmd} - +
))}
)} @@ -2084,18 +1979,14 @@ const AgentSection = ({ onKeyDown={(e) => { if (e.key === 'Enter') { e.preventDefault(); - setBlockedCommands((s) => addUniqueItem(s, blockedCommandDraft)); - setBlockedCommandDraft(''); + handleAddBlockedCommand(); } }} />