diff --git a/rust/crates/compat-harness/src/lib.rs b/rust/crates/compat-harness/src/lib.rs index 1acfec9..225a73c 100644 --- a/rust/crates/compat-harness/src/lib.rs +++ b/rust/crates/compat-harness/src/lib.rs @@ -18,6 +18,12 @@ impl UpstreamPaths { } } + /// Returns the repository root path. + #[must_use] + pub fn repo_root(&self) -> &Path { + &self.repo_root + } + #[must_use] pub fn from_workspace_dir(workspace_dir: impl AsRef) -> Self { let workspace_dir = workspace_dir diff --git a/rust/crates/plugins/src/lib.rs b/rust/crates/plugins/src/lib.rs index 052d1ba..bebd17e 100644 --- a/rust/crates/plugins/src/lib.rs +++ b/rust/crates/plugins/src/lib.rs @@ -1,4 +1,6 @@ mod hooks; +#[cfg(test)] +pub mod test_isolation; use std::collections::{BTreeMap, BTreeSet}; use std::fmt::{Display, Formatter}; @@ -2160,7 +2162,13 @@ fn materialize_source( match source { PluginInstallSource::LocalPath { path } => Ok(path.clone()), PluginInstallSource::GitUrl { url } => { - let destination = temp_root.join(format!("plugin-{}", unix_time_ms())); + static MATERIALIZE_COUNTER: AtomicU64 = AtomicU64::new(0); + let unique = MATERIALIZE_COUNTER.fetch_add(1, Ordering::Relaxed); + let nanos = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos(); + let destination = temp_root.join(format!("plugin-{nanos}-{unique}")); let output = Command::new("git") .arg("clone") .arg("--depth") @@ -2273,6 +2281,14 @@ fn ensure_object<'a>(root: &'a mut Map, key: &str) -> &'a mut Map .expect("object should exist") } +/// Environment variable lock for test isolation. +/// Guards against concurrent modification of `CLAW_CONFIG_HOME`. +#[cfg(test)] +fn env_lock() -> &'static std::sync::Mutex<()> { + static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + &ENV_LOCK +} + #[cfg(test)] mod tests { use super::*; @@ -2468,6 +2484,7 @@ mod tests { #[test] fn load_plugin_from_directory_validates_required_fields() { + let _guard = env_lock().lock().expect("env lock"); let root = temp_dir("manifest-required"); write_file( root.join(MANIFEST_FILE_NAME).as_path(), @@ -2482,6 +2499,7 @@ mod tests { #[test] fn load_plugin_from_directory_reads_root_manifest_and_validates_entries() { + let _guard = env_lock().lock().expect("env lock"); let root = temp_dir("manifest-root"); write_loader_plugin(&root); @@ -2511,6 +2529,7 @@ mod tests { #[test] fn load_plugin_from_directory_supports_packaged_manifest_path() { + let _guard = env_lock().lock().expect("env lock"); let root = temp_dir("manifest-packaged"); write_external_plugin(&root, "packaged-demo", "1.0.0"); @@ -2524,6 +2543,7 @@ mod tests { #[test] fn load_plugin_from_directory_defaults_optional_fields() { + let _guard = env_lock().lock().expect("env lock"); let root = temp_dir("manifest-defaults"); write_file( root.join(MANIFEST_FILE_NAME).as_path(), @@ -2545,6 +2565,7 @@ mod tests { #[test] fn load_plugin_from_directory_rejects_duplicate_permissions_and_commands() { + let _guard = env_lock().lock().expect("env lock"); let root = temp_dir("manifest-duplicates"); write_file( root.join("commands").join("sync.sh").as_path(), @@ -2840,6 +2861,7 @@ mod tests { #[test] fn discovers_builtin_and_bundled_plugins() { + let _guard = env_lock().lock().expect("env lock"); let manager = PluginManager::new(PluginManagerConfig::new(temp_dir("discover"))); let plugins = manager.list_plugins().expect("plugins should list"); assert!(plugins @@ -2852,6 +2874,7 @@ mod tests { #[test] fn installs_enables_updates_and_uninstalls_external_plugins() { + let _guard = env_lock().lock().expect("env lock"); let config_home = temp_dir("home"); let source_root = temp_dir("source"); write_external_plugin(&source_root, "demo", "1.0.0"); @@ -2900,6 +2923,7 @@ mod tests { #[test] fn auto_installs_bundled_plugins_into_the_registry() { + let _guard = env_lock().lock().expect("env lock"); let config_home = temp_dir("bundled-home"); let bundled_root = temp_dir("bundled-root"); write_bundled_plugin(&bundled_root.join("starter"), "starter", "0.1.0", false); @@ -2931,6 +2955,7 @@ mod tests { #[test] fn default_bundled_root_loads_repo_bundles_as_installed_plugins() { + let _guard = env_lock().lock().expect("env lock"); let config_home = temp_dir("default-bundled-home"); let manager = PluginManager::new(PluginManagerConfig::new(&config_home)); @@ -2949,6 +2974,7 @@ mod tests { #[test] fn bundled_sync_prunes_removed_bundled_registry_entries() { + let _guard = env_lock().lock().expect("env lock"); let config_home = temp_dir("bundled-prune-home"); let bundled_root = temp_dir("bundled-prune-root"); let stale_install_path = config_home @@ -3012,6 +3038,7 @@ mod tests { #[test] fn installed_plugin_discovery_keeps_registry_entries_outside_install_root() { + let _guard = env_lock().lock().expect("env lock"); let config_home = temp_dir("registry-fallback-home"); let bundled_root = temp_dir("registry-fallback-bundled"); let install_root = config_home.join("plugins").join("installed"); @@ -3066,6 +3093,7 @@ mod tests { #[test] fn installed_plugin_discovery_prunes_stale_registry_entries() { + let _guard = env_lock().lock().expect("env lock"); let config_home = temp_dir("registry-prune-home"); let bundled_root = temp_dir("registry-prune-bundled"); let install_root = config_home.join("plugins").join("installed"); @@ -3111,6 +3139,7 @@ mod tests { #[test] fn persists_bundled_plugin_enable_state_across_reloads() { + let _guard = env_lock().lock().expect("env lock"); let config_home = temp_dir("bundled-state-home"); let bundled_root = temp_dir("bundled-state-root"); write_bundled_plugin(&bundled_root.join("starter"), "starter", "0.1.0", false); @@ -3144,6 +3173,7 @@ mod tests { #[test] fn persists_bundled_plugin_disable_state_across_reloads() { + let _guard = env_lock().lock().expect("env lock"); let config_home = temp_dir("bundled-disabled-home"); let bundled_root = temp_dir("bundled-disabled-root"); write_bundled_plugin(&bundled_root.join("starter"), "starter", "0.1.0", true); @@ -3177,6 +3207,7 @@ mod tests { #[test] fn validates_plugin_source_before_install() { + let _guard = env_lock().lock().expect("env lock"); let config_home = temp_dir("validate-home"); let source_root = temp_dir("validate-source"); write_external_plugin(&source_root, "validator", "1.0.0"); @@ -3191,6 +3222,7 @@ mod tests { #[test] fn plugin_registry_tracks_enabled_state_and_lookup() { + let _guard = env_lock().lock().expect("env lock"); let config_home = temp_dir("registry-home"); let source_root = temp_dir("registry-source"); write_external_plugin(&source_root, "registry-demo", "1.0.0"); @@ -3218,6 +3250,7 @@ mod tests { #[test] fn plugin_registry_report_collects_load_failures_without_dropping_valid_plugins() { + let _guard = env_lock().lock().expect("env lock"); // given let config_home = temp_dir("report-home"); let external_root = temp_dir("report-external"); @@ -3262,6 +3295,7 @@ mod tests { #[test] fn installed_plugin_registry_report_collects_load_failures_from_install_root() { + let _guard = env_lock().lock().expect("env lock"); // given let config_home = temp_dir("installed-report-home"); let bundled_root = temp_dir("installed-report-bundled"); @@ -3292,6 +3326,7 @@ mod tests { #[test] fn rejects_plugin_sources_with_missing_hook_paths() { + let _guard = env_lock().lock().expect("env lock"); // given let config_home = temp_dir("broken-home"); let source_root = temp_dir("broken-source"); @@ -3319,6 +3354,7 @@ mod tests { #[test] fn rejects_plugin_sources_with_missing_failure_hook_paths() { + let _guard = env_lock().lock().expect("env lock"); // given let config_home = temp_dir("broken-failure-home"); let source_root = temp_dir("broken-failure-source"); @@ -3346,6 +3382,7 @@ mod tests { #[test] fn plugin_registry_runs_initialize_and_shutdown_for_enabled_plugins() { + let _guard = env_lock().lock().expect("env lock"); let config_home = temp_dir("lifecycle-home"); let source_root = temp_dir("lifecycle-source"); let _ = write_lifecycle_plugin(&source_root, "lifecycle-demo", "1.0.0"); @@ -3369,6 +3406,7 @@ mod tests { #[test] fn aggregates_and_executes_plugin_tools() { + let _guard = env_lock().lock().expect("env lock"); let config_home = temp_dir("tool-home"); let source_root = temp_dir("tool-source"); write_tool_plugin(&source_root, "tool-demo", "1.0.0"); @@ -3397,6 +3435,7 @@ mod tests { #[test] fn list_installed_plugins_scans_install_root_without_registry_entries() { + let _guard = env_lock().lock().expect("env lock"); let config_home = temp_dir("installed-scan-home"); let bundled_root = temp_dir("installed-scan-bundled"); let install_root = config_home.join("plugins").join("installed"); @@ -3428,6 +3467,7 @@ mod tests { #[test] fn list_installed_plugins_scans_packaged_manifests_in_install_root() { + let _guard = env_lock().lock().expect("env lock"); let config_home = temp_dir("installed-packaged-scan-home"); let bundled_root = temp_dir("installed-packaged-scan-bundled"); let install_root = config_home.join("plugins").join("installed"); @@ -3456,4 +3496,143 @@ mod tests { let _ = fs::remove_dir_all(config_home); let _ = fs::remove_dir_all(bundled_root); } + + /// Regression test for ROADMAP #41: verify that `CLAW_CONFIG_HOME` isolation prevents + /// host `~/.claw/plugins/` from bleeding into test runs. + #[test] + fn claw_config_home_isolation_prevents_host_plugin_leakage() { + let _guard = env_lock().lock().expect("env lock"); + + // Create a temp directory to act as our isolated CLAW_CONFIG_HOME + let config_home = temp_dir("isolated-home"); + let bundled_root = temp_dir("isolated-bundled"); + + // Set CLAW_CONFIG_HOME to our temp directory + std::env::set_var("CLAW_CONFIG_HOME", &config_home); + + // Create a test fixture plugin in the isolated config home + let install_root = config_home.join("plugins").join("installed"); + let fixture_plugin_root = install_root.join("isolated-test-plugin"); + write_file( + fixture_plugin_root.join(MANIFEST_RELATIVE_PATH).as_path(), + r#"{ + "name": "isolated-test-plugin", + "version": "1.0.0", + "description": "Test fixture plugin in isolated config home" +}"#, + ); + + // Create PluginManager with isolated bundled_root - it should use the temp config_home, not host ~/.claw/ + let mut config = PluginManagerConfig::new(&config_home); + config.bundled_root = Some(bundled_root.clone()); + let manager = PluginManager::new(config); + + // List installed plugins - should only see the test fixture, not host plugins + let installed = manager + .list_installed_plugins() + .expect("installed plugins should list"); + + // Verify we only see the test fixture plugin + assert_eq!( + installed.len(), + 1, + "should only see the test fixture plugin, not host ~/.claw/plugins/" + ); + assert_eq!( + installed[0].metadata.id, "isolated-test-plugin@external", + "should see the test fixture plugin" + ); + + // Cleanup + std::env::remove_var("CLAW_CONFIG_HOME"); + let _ = fs::remove_dir_all(config_home); + let _ = fs::remove_dir_all(bundled_root); + } + + #[test] + fn plugin_lifecycle_handles_parallel_execution() { + use std::sync::atomic::{AtomicUsize, Ordering as AtomicOrdering}; + use std::sync::Arc; + use std::thread; + + let _guard = env_lock().lock().expect("env lock"); + + // Shared base directory for all threads + let base_dir = temp_dir("parallel-base"); + + // Track successful installations and any errors + let success_count = Arc::new(AtomicUsize::new(0)); + let error_count = Arc::new(AtomicUsize::new(0)); + + // Spawn multiple threads to install plugins simultaneously + let mut handles = Vec::new(); + for thread_id in 0..5 { + let base_dir = base_dir.clone(); + let success_count = Arc::clone(&success_count); + let error_count = Arc::clone(&error_count); + + let handle = thread::spawn(move || { + // Create unique directories for this thread + let config_home = base_dir.join(format!("config-{thread_id}")); + let source_root = base_dir.join(format!("source-{thread_id}")); + + // Write lifecycle plugin for this thread + let _log_path = + write_lifecycle_plugin(&source_root, &format!("parallel-{thread_id}"), "1.0.0"); + + // Create PluginManager and install + let mut manager = PluginManager::new(PluginManagerConfig::new(&config_home)); + let install_result = manager.install(source_root.to_str().expect("utf8 path")); + + match install_result { + Ok(install) => { + let log_path = install.install_path.join("lifecycle.log"); + + // Initialize and shutdown the registry to trigger lifecycle hooks + let registry = manager.plugin_registry(); + match registry { + Ok(registry) => { + if registry.initialize().is_ok() && registry.shutdown().is_ok() { + // Verify lifecycle.log exists and has expected content + if let Ok(log) = fs::read_to_string(&log_path) { + if log == "init\nshutdown\n" { + success_count.fetch_add(1, AtomicOrdering::Relaxed); + } + } + } + } + Err(_) => { + error_count.fetch_add(1, AtomicOrdering::Relaxed); + } + } + } + Err(_) => { + error_count.fetch_add(1, AtomicOrdering::Relaxed); + } + } + }); + handles.push(handle); + } + + // Wait for all threads to complete + for handle in handles { + handle.join().expect("thread should complete"); + } + + // Verify all threads succeeded without collisions + let successes = success_count.load(AtomicOrdering::Relaxed); + let errors = error_count.load(AtomicOrdering::Relaxed); + + assert_eq!( + successes, 5, + "all 5 parallel plugin installations should succeed" + ); + assert_eq!( + errors, 0, + "no errors should occur during parallel execution" + ); + + // Cleanup + let _ = fs::remove_dir_all(base_dir); + } } diff --git a/rust/crates/plugins/src/test_isolation.rs b/rust/crates/plugins/src/test_isolation.rs new file mode 100644 index 0000000..2e4e7b8 --- /dev/null +++ b/rust/crates/plugins/src/test_isolation.rs @@ -0,0 +1,72 @@ +// Test isolation utilities for plugin tests +// ROADMAP #41: Stop ambient plugin state from skewing CLI regression checks + +use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::Mutex; +use std::env; +use std::path::PathBuf; + +static TEST_COUNTER: AtomicU64 = AtomicU64::new(0); +static ENV_LOCK: Mutex<()> = Mutex::new(()); + +/// Lock for test environment isolation +pub struct EnvLock { + _guard: std::sync::MutexGuard<'static, ()>, + temp_home: PathBuf, +} + +impl EnvLock { + /// Acquire environment lock for test isolation + pub fn lock() -> Self { + let guard = ENV_LOCK.lock().unwrap(); + let count = TEST_COUNTER.fetch_add(1, Ordering::SeqCst); + let temp_home = std::env::temp_dir().join(format!("plugin-test-{}", count)); + + // Set up isolated environment + std::fs::create_dir_all(&temp_home).ok(); + std::fs::create_dir_all(temp_home.join(".claude/plugins/installed")).ok(); + std::fs::create_dir_all(temp_home.join(".config")).ok(); + + // Redirect HOME and XDG_CONFIG_HOME to temp directory + env::set_var("HOME", &temp_home); + env::set_var("XDG_CONFIG_HOME", temp_home.join(".config")); + env::set_var("XDG_DATA_HOME", temp_home.join(".local/share")); + + EnvLock { + _guard: guard, + temp_home, + } + } + + /// Get the temporary home directory for this test + pub fn temp_home(&self) -> &PathBuf { + &self.temp_home + } +} + +impl Drop for EnvLock { + fn drop(&mut self) { + // Cleanup temp directory + std::fs::remove_dir_all(&self.temp_home).ok(); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_env_lock_creates_isolated_home() { + let lock = EnvLock::lock(); + let home = env::var("HOME").unwrap(); + assert!(home.contains("plugin-test-")); + assert_eq!(home, lock.temp_home().to_str().unwrap()); + } + + #[test] + fn test_env_lock_creates_plugin_directories() { + let lock = EnvLock::lock(); + let plugins_dir = lock.temp_home().join(".claude/plugins/installed"); + assert!(plugins_dir.exists()); + } +} diff --git a/rust/crates/runtime/src/permission_enforcer.rs b/rust/crates/runtime/src/permission_enforcer.rs index 3a45dc8..6ff872b 100644 --- a/rust/crates/runtime/src/permission_enforcer.rs +++ b/rust/crates/runtime/src/permission_enforcer.rs @@ -65,6 +65,40 @@ impl PermissionEnforcer { matches!(self.check(tool_name, input), EnforcementResult::Allowed) } + /// Check permission with an explicitly provided required mode. + /// Used when the required mode is determined dynamically (e.g., bash command classification). + pub fn check_with_required_mode( + &self, + tool_name: &str, + input: &str, + required_mode: PermissionMode, + ) -> EnforcementResult { + // When the active mode is Prompt, defer to the caller's interactive + // prompt flow rather than hard-denying. + if self.policy.active_mode() == PermissionMode::Prompt { + return EnforcementResult::Allowed; + } + + let active_mode = self.policy.active_mode(); + + // Check if active mode meets the dynamically determined required mode + if active_mode >= required_mode { + return EnforcementResult::Allowed; + } + + // Permission denied - active mode is insufficient + EnforcementResult::Denied { + tool: tool_name.to_owned(), + active_mode: active_mode.as_str().to_owned(), + required_mode: required_mode.as_str().to_owned(), + reason: format!( + "'{tool_name}' with input '{input}' requires '{}' permission, but current mode is '{}'", + required_mode.as_str(), + active_mode.as_str() + ), + } + } + #[must_use] pub fn active_mode(&self) -> PermissionMode { self.policy.active_mode() diff --git a/rust/crates/rusty-claude-cli/src/main.rs b/rust/crates/rusty-claude-cli/src/main.rs index 5bd93ca..622e06d 100644 --- a/rust/crates/rusty-claude-cli/src/main.rs +++ b/rust/crates/rusty-claude-cli/src/main.rs @@ -1901,14 +1901,34 @@ fn looks_like_slash_command_token(token: &str) -> bool { fn dump_manifests(output_format: CliOutputFormat) -> Result<(), Box> { let workspace_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../.."); + dump_manifests_at_path(&workspace_dir, output_format) +} + +// Internal function for testing that accepts a workspace directory path. +fn dump_manifests_at_path( + workspace_dir: &std::path::Path, + output_format: CliOutputFormat, +) -> Result<(), Box> { // Surface the resolved path in the error so users can diagnose missing // manifest files without guessing what path the binary expected. // ROADMAP #45: this path is only correct when running from the build tree; // a proper fix would ship manifests alongside the binary. let resolved = workspace_dir .canonicalize() - .unwrap_or_else(|_| workspace_dir.clone()); - let paths = UpstreamPaths::from_workspace_dir(&workspace_dir); + .unwrap_or_else(|_| workspace_dir.to_path_buf()); + + let paths = UpstreamPaths::from_workspace_dir(&resolved); + + // Pre-check: verify manifest directory exists + let manifest_dir = paths.repo_root(); + if !manifest_dir.exists() { + return Err(format!( + "Manifest files (commands.ts, tools.ts) define CLI commands and tools.\n Expected at: {}\n Run `claw init` to create them or specify --manifests-dir.", + manifest_dir.display() + ) + .into()); + } + match extract_manifest(&paths) { Ok(manifest) => { match output_format { @@ -1930,8 +1950,8 @@ fn dump_manifests(output_format: CliOutputFormat) -> Result<(), Box Err(format!( - "failed to extract manifests: {error}\n looked in: {}", - resolved.display() + "failed to extract manifests: {error}\n looked in: {path}", + path = paths.repo_root().display() ) .into()), } @@ -11481,3 +11501,46 @@ mod sandbox_report_tests { assert!(abort_signal.is_aborted()); } } + +#[cfg(test)] +mod dump_manifests_tests { + use super::{dump_manifests_at_path, CliOutputFormat}; + + #[test] + fn dump_manifests_shows_helpful_error_when_manifests_missing() { + // Create a temp directory without manifest files + let temp_dir = std::env::temp_dir().join(format!( + "claw_test_missing_manifests_{}", + std::process::id() + )); + std::fs::create_dir_all(&temp_dir).expect("failed to create temp dir"); + + // Clean up at the end of the test + let _cleanup = std::panic::catch_unwind(|| { + // Call dump_manifests_at_path with the temp directory + let result = dump_manifests_at_path(&temp_dir, CliOutputFormat::Text); + + // Assert that the call fails + assert!(result.is_err(), "expected an error when manifests are missing"); + + let error_msg = result.unwrap_err().to_string(); + + // Assert the error message contains "Manifest files (commands.ts, tools.ts)" + assert!( + error_msg.contains("Manifest files (commands.ts, tools.ts)"), + "error message should mention manifest files: {}", + error_msg + ); + + // Assert the error message contains the expected path + assert!( + error_msg.contains(&temp_dir.display().to_string()), + "error message should contain the expected path: {}", + error_msg + ); + }); + + // Clean up temp directory + let _ = std::fs::remove_dir_all(&temp_dir); + } +} diff --git a/rust/crates/tools/src/lib.rs b/rust/crates/tools/src/lib.rs index cffd68e..07799f6 100644 --- a/rust/crates/tools/src/lib.rs +++ b/rust/crates/tools/src/lib.rs @@ -1182,8 +1182,11 @@ fn execute_tool_with_enforcer( ) -> Result { match name { "bash" => { - maybe_enforce_permission_check(enforcer, name, input)?; - from_value::(input).and_then(run_bash) + // Parse input to get the command for permission classification + let bash_input: BashCommandInput = from_value(input)?; + let classified_mode = classify_bash_permission(&bash_input.command); + maybe_enforce_permission_check_with_mode(enforcer, name, input, classified_mode)?; + run_bash(bash_input) } "read_file" => { maybe_enforce_permission_check(enforcer, name, input)?; @@ -1221,7 +1224,13 @@ fn execute_tool_with_enforcer( from_value::(input).and_then(run_structured_output) } "REPL" => from_value::(input).and_then(run_repl), - "PowerShell" => from_value::(input).and_then(run_powershell), + "PowerShell" => { + // Parse input to get the command for permission classification + let ps_input: PowerShellInput = from_value(input)?; + let classified_mode = classify_powershell_permission(&ps_input.command); + maybe_enforce_permission_check_with_mode(enforcer, name, input, classified_mode)?; + run_powershell(ps_input) + } "AskUserQuestion" => { from_value::(input).and_then(run_ask_user_question) } @@ -1277,6 +1286,28 @@ fn maybe_enforce_permission_check( Ok(()) } +/// Enforce permission check with a dynamically classified permission mode. +/// Used for tools like bash and PowerShell where the required permission +/// depends on the actual command being executed. +fn maybe_enforce_permission_check_with_mode( + enforcer: Option<&PermissionEnforcer>, + tool_name: &str, + input: &Value, + required_mode: PermissionMode, +) -> Result<(), String> { + if let Some(enforcer) = enforcer { + let input_str = serde_json::to_string(input).unwrap_or_default(); + let result = enforcer.check_with_required_mode(tool_name, &input_str, required_mode); + + match result { + EnforcementResult::Allowed => Ok(()), + EnforcementResult::Denied { reason, .. } => Err(reason), + } + } else { + Ok(()) + } +} + #[allow(clippy::needless_pass_by_value)] fn run_ask_user_question(input: AskUserQuestionInput) -> Result { use std::io::{self, BufRead, Write}; @@ -1788,6 +1819,73 @@ fn from_value Deserialize<'de>>(input: &Value) -> Result serde_json::from_value(input.clone()).map_err(|error| error.to_string()) } +/// Classify bash command permission based on command type and path. +/// ROADMAP #50: Read-only commands targeting CWD paths get WorkspaceWrite, +/// all others remain DangerFullAccess. +fn classify_bash_permission(command: &str) -> PermissionMode { + // Read-only commands that are safe when targeting workspace paths + const READ_ONLY_COMMANDS: &[&str] = &[ + "cat", "head", "tail", "less", "more", "ls", "ll", "dir", "find", "test", "[", "[[", + "grep", "rg", "awk", "sed", "file", "stat", "readlink", "wc", "sort", "uniq", "cut", "tr", + "pwd", "echo", "printf", + ]; + + // Get the base command (first word before any args or pipes) + let base_cmd = command.trim().split_whitespace().next().unwrap_or(""); + let base_cmd = base_cmd.split('|').next().unwrap_or("").trim(); + let base_cmd = base_cmd.split(';').next().unwrap_or("").trim(); + let base_cmd = base_cmd.split('>').next().unwrap_or("").trim(); + let base_cmd = base_cmd.split('<').next().unwrap_or("").trim(); + + // Check if it's a read-only command + let cmd_name = base_cmd.split('/').last().unwrap_or(base_cmd); + let is_read_only = READ_ONLY_COMMANDS.contains(&cmd_name); + + if !is_read_only { + return PermissionMode::DangerFullAccess; + } + + // Check if any path argument is outside workspace + // Simple heuristic: check for absolute paths not starting with CWD + if has_dangerous_paths(command) { + return PermissionMode::DangerFullAccess; + } + + PermissionMode::WorkspaceWrite +} + +/// Check if command has dangerous paths (outside workspace). +fn has_dangerous_paths(command: &str) -> bool { + // Look for absolute paths + let tokens: Vec<&str> = command.split_whitespace().collect(); + + for token in tokens { + // Skip flags/options + if token.starts_with('-') { + continue; + } + + // Check for absolute paths + if token.starts_with('/') || token.starts_with("~/") { + // Check if it's within CWD + let path = + PathBuf::from(token.replace("~", &std::env::var("HOME").unwrap_or_default())); + if let Ok(cwd) = std::env::current_dir() { + if !path.starts_with(&cwd) { + return true; // Path outside workspace + } + } + } + + // Check for parent directory traversal that escapes workspace + if token.contains("../..") || token.starts_with("../") && !token.starts_with("./") { + return true; + } + } + + false +} + fn run_bash(input: BashCommandInput) -> Result { if let Some(output) = workspace_test_branch_preflight(&input.command) { return serde_json::to_string_pretty(&output).map_err(|error| error.to_string()); @@ -2033,6 +2131,78 @@ fn run_repl(input: ReplInput) -> Result { to_pretty_json(execute_repl(input)?) } +/// Classify PowerShell command permission based on command type and path. +/// ROADMAP #50: Read-only commands targeting CWD paths get WorkspaceWrite, +/// all others remain DangerFullAccess. +fn classify_powershell_permission(command: &str) -> PermissionMode { + // Read-only commands that are safe when targeting workspace paths + const READ_ONLY_COMMANDS: &[&str] = &[ + "Get-Content", + "Get-ChildItem", + "Test-Path", + "Get-Item", + "Get-ItemProperty", + "Get-FileHash", + "Select-String", + ]; + + // Check if command starts with a read-only cmdlet + let cmd_lower = command.trim().to_lowercase(); + let is_read_only_cmd = READ_ONLY_COMMANDS + .iter() + .any(|cmd| cmd_lower.starts_with(&cmd.to_lowercase())); + + if !is_read_only_cmd { + return PermissionMode::DangerFullAccess; + } + + // Check if the path is within workspace (CWD or subdirectory) + // Extract path from command - look for -Path or positional parameter + let path = extract_powershell_path(command); + match path { + Some(p) if is_within_workspace(&p) => PermissionMode::WorkspaceWrite, + _ => PermissionMode::DangerFullAccess, + } +} + +/// Extract the path argument from a PowerShell command. +fn extract_powershell_path(command: &str) -> Option { + // Look for -Path parameter + if let Some(idx) = command.to_lowercase().find("-path") { + let after_path = &command[idx + 5..]; + let path = after_path.trim().split_whitespace().next()?; + return Some(path.trim_matches('"').trim_matches('\'').to_string()); + } + + // Look for positional path parameter (after command name) + let parts: Vec<&str> = command.trim().split_whitespace().collect(); + if parts.len() >= 2 { + // Skip the cmdlet name and take the first argument + let first_arg = parts[1]; + // Check if it looks like a path (contains \, /, or .) + if first_arg.contains(['\\', '/', '.']) { + return Some(first_arg.trim_matches('"').trim_matches('\'').to_string()); + } + } + + None +} + +/// Check if a path is within the current workspace. +fn is_within_workspace(path: &str) -> bool { + let path = PathBuf::from(path); + + // If path is absolute, check if it starts with CWD + if path.is_absolute() { + if let Ok(cwd) = std::env::current_dir() { + return path.starts_with(&cwd); + } + } + + // Relative paths are assumed to be within workspace + !path.starts_with("/") && !path.starts_with("\\") && !path.starts_with("..") +} + fn run_powershell(input: PowerShellInput) -> Result { to_pretty_json(execute_powershell(input).map_err(|error| error.to_string())?) } @@ -8258,11 +8428,12 @@ printf 'pwsh:%s' "$1" #[test] fn given_read_only_enforcer_when_bash_then_denied() { let registry = read_only_registry(); + // Use a command that requires DangerFullAccess (rm) to ensure it's blocked in read-only mode let err = registry - .execute("bash", &json!({ "command": "echo hi" })) + .execute("bash", &json!({ "command": "rm -rf /" })) .expect_err("bash should be denied in read-only mode"); assert!( - err.contains("current mode is read-only"), + err.contains("current mode is 'read-only'"), "should cite active mode: {err}" ); }