mirror of
https://github.com/ultraworkers/claw-code.git
synced 2026-04-12 20:01:54 +08:00
Merged late-arriving droid output from 10 parallel ultraclaw sessions. ROADMAP #41 — Test isolation for plugin regression checks: - Add test_isolation.rs module with env_lock() for test environment isolation - Redirect HOME/XDG_CONFIG_HOME/XDG_DATA_HOME to unique temp dirs per test - Prevent host ~/.claude/plugins/ from bleeding into test runs - Auto-cleanup temp directories on drop via RAII pattern - Tests: 39 plugin tests passing ROADMAP #50 — PowerShell workspace-aware permissions: - Add is_safe_powershell_command() for command-level permission analysis - Add is_path_within_workspace() for workspace boundary validation - Classify read-only vs write-requiring bash commands (60+ commands) - Dynamic permission requirements based on command type and target path - Tests: permission enforcer and workspace boundary tests passing Additional improvements: - runtime/src/permission_enforcer.rs: Dynamic permission enforcement layer - check_with_required_mode() for dynamically-determined permissions - 60+ read-only command patterns (cat, find, grep, cargo, git, jq, yq, etc.) - Workspace-path detection for safe commands - compat-harness/src/lib.rs: Compat harness updates for permission testing - rusty-claude-cli/src/main.rs: CLI integration for permission modes - plugins/src/lib.rs: Updated imports for test isolation module Total: +410 lines across 5 files Workspace tests: 448+ passed Droid source: ultraclaw-04-test-isolation, ultraclaw-08-powershell-permissions Ultraclaw total: 4 ROADMAP items committed (38, 40, 41, 50)
This commit is contained in:
YeonGyu-Kim
parent
723e2117af
commit
16b9febdae
@ -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<Path>) -> Self {
|
||||
let workspace_dir = workspace_dir
|
||||
|
||||
@ -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<String, Value>, 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);
|
||||
}
|
||||
}
|
||||
|
||||
72
rust/crates/plugins/src/test_isolation.rs
Normal file
72
rust/crates/plugins/src/test_isolation.rs
Normal file
@ -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());
|
||||
}
|
||||
}
|
||||
@ -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()
|
||||
|
||||
@ -1901,14 +1901,34 @@ fn looks_like_slash_command_token(token: &str) -> bool {
|
||||
|
||||
fn dump_manifests(output_format: CliOutputFormat) -> Result<(), Box<dyn std::error::Error>> {
|
||||
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<dyn std::error::Error>> {
|
||||
// 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<dyn std::err
|
||||
Ok(())
|
||||
}
|
||||
Err(error) => 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);
|
||||
}
|
||||
}
|
||||
|
||||
@ -1182,8 +1182,11 @@ fn execute_tool_with_enforcer(
|
||||
) -> Result<String, String> {
|
||||
match name {
|
||||
"bash" => {
|
||||
maybe_enforce_permission_check(enforcer, name, input)?;
|
||||
from_value::<BashCommandInput>(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::<StructuredOutputInput>(input).and_then(run_structured_output)
|
||||
}
|
||||
"REPL" => from_value::<ReplInput>(input).and_then(run_repl),
|
||||
"PowerShell" => from_value::<PowerShellInput>(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::<AskUserQuestionInput>(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<String, String> {
|
||||
use std::io::{self, BufRead, Write};
|
||||
@ -1788,6 +1819,73 @@ fn from_value<T: for<'de> Deserialize<'de>>(input: &Value) -> Result<T, String>
|
||||
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<String, String> {
|
||||
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<String, String> {
|
||||
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<String> {
|
||||
// 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<String, String> {
|
||||
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}"
|
||||
);
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user