diff --git a/cargo_crap_baseline.json b/cargo_crap_baseline.json index 8f00cb7..f5a36f5 100644 --- a/cargo_crap_baseline.json +++ b/cargo_crap_baseline.json @@ -42,6 +42,14 @@ "coverage": 69.93464052287581, "crap": 93.25000551433072 }, + { + "file": "./src/main.rs", + "function": "run_command", + "line": 172, + "cyclomatic": 10.0, + "coverage": 21.428571428571427, + "crap": 58.505830903790084 + }, { "file": "./src/prompt.rs", "function": "prompt_for_task", @@ -58,14 +66,6 @@ "coverage": 55.55555555555556, "crap": 50.692729766803836 }, - { - "file": "./src/main.rs", - "function": "run_command", - "line": 162, - "cyclomatic": 9.0, - "coverage": 22.22222222222222, - "crap": 47.111111111111114 - }, { "file": "./src/mcp/job_manager.rs", "function": "JobManager::garbage_collect", @@ -239,8 +239,8 @@ "function": "evaluate_task_against_allowlist", "line": 75, "cyclomatic": 14.0, - "coverage": 100.0, - "crap": 14.0 + "coverage": 95.65217391304348, + "crap": 14.016109147694584 }, { "file": "./src/mcp/server.rs", @@ -293,7 +293,7 @@ { "file": "./src/main.rs", "function": "main", - "line": 198, + "line": 209, "cyclomatic": 3.0, "coverage": 0.0, "crap": 12.0 @@ -674,6 +674,14 @@ "coverage": 0.0, "crap": 6.0 }, + { + "file": "./src/commands/allow.rs", + "function": "execute", + "line": 7, + "cyclomatic": 6.0, + "coverage": 100.0, + "crap": 6.0 + }, { "file": "./src/commands/mcp.rs", "function": "Editor::config_path", diff --git a/dev_docs/project_plan.md b/dev_docs/project_plan.md index 041316a..b350743 100644 --- a/dev_docs/project_plan.md +++ b/dev_docs/project_plan.md @@ -130,7 +130,7 @@ This plan outlines the major development phases and tasks for building `dela`, a - [x] [DTKT-31] Support "Allow once," "Allow this task," "Allow file," "Allow directory," and "Deny." - [x] [DTKT-32] Persist decisions in the allowlist. - [ ] [DTKT-33] Have `dela run` take an optional `--allow` flag to allow a task without prompting. - - [ ] [DTKT-109] Implement `dela allow` command to add allowlist entries. + - [x] [DTKT-109] Implement `dela allow` command to add allowlist entries. - [ ] [DTKT-110] Implement `dela deny` command to add denylist entries. - [ ] [DTKT-111] Implement `dela run --allow-once` command to run a command once. diff --git a/src/commands/allow.rs b/src/commands/allow.rs new file mode 100644 index 0000000..00387e5 --- /dev/null +++ b/src/commands/allow.rs @@ -0,0 +1,218 @@ +use crate::allowlist; +use crate::task_discovery; +use crate::types::AllowScope; +use std::env; + +/// Executes the 'dela allow' command to add a specific task to the allowlist. +pub fn execute(task_name: &str) -> anyhow::Result<()> { + let current_dir = env::current_dir() + .map_err(|e| anyhow::anyhow!("Failed to get current directory: {}", e))?; + let discovered = task_discovery::discover_tasks(¤t_dir); + + // Find all tasks with the given name (both original and disambiguated) + let matching_tasks = task_discovery::get_matching_tasks(&discovered, task_name); + + match matching_tasks.len() { + 0 => Err(anyhow::anyhow!( + "dela: command or task not found: {}", + task_name + )), + 1 => { + let task = matching_tasks[0]; + allowlist::check_task_allowed_with_scope(task, AllowScope::Task)?; + println!( + "Added task '{}' ({}) to allowlist.", + task.name, + task.runner.short_name() + ); + Ok(()) + } + _ => { + let error_msg = task_discovery::format_ambiguous_task_error(task_name, &matching_tasks); + eprintln!("{}", error_msg); + Err(anyhow::anyhow!( + "Multiple tasks named '{}' found", + task_name + )) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::{preferred_allowlist_path_for, preferred_config_dir_path_for}; + use crate::environment::{TestEnvironment, reset_to_real_environment, set_test_environment}; + use serial_test::serial; + use std::env; + use std::fs::{self, File}; + use std::io::Write; + use tempfile::TempDir; + + struct TestEnvGuard { + project_dir: TempDir, + home_dir: TempDir, + } + + impl TestEnvGuard { + fn new() -> Self { + let (project_dir, home_dir) = setup_test_env(); + Self { + project_dir, + home_dir, + } + } + + fn new_uninitialized() -> Self { + // Create a temp dir for HOME but don't create the dela config directory + let home_dir = TempDir::new().expect("Failed to create temp HOME directory"); + + // Set up test environment with the temp directory as HOME + let test_env = TestEnvironment::new().with_home(home_dir.path().to_string_lossy()); + set_test_environment(test_env); + + Self { + project_dir: TempDir::new().expect("Failed to create temp directory"), + home_dir, + } + } + } + + impl Drop for TestEnvGuard { + fn drop(&mut self) { + reset_to_real_environment(); + } + } + + fn setup_test_env() -> (TempDir, TempDir) { + // Create a temp dir for the project + let project_dir = TempDir::new().expect("Failed to create temp directory"); + + // Create a test Makefile + let makefile_content = " +build: ## Building the project +\t@echo Building... + +test: ## Running tests +\t@echo Testing... +"; + let mut makefile = + File::create(project_dir.path().join("Makefile")).expect("Failed to create Makefile"); + makefile + .write_all(makefile_content.as_bytes()) + .expect("Failed to write Makefile"); + + // Create a temp dir for HOME and set it up + let home_dir = TempDir::new().expect("Failed to create temp HOME directory"); + + // Set up test environment with the temp directory as HOME + let test_env = TestEnvironment::new().with_home(home_dir.path().to_string_lossy()); + set_test_environment(test_env); + + // Create ~/.config/dela directory + fs::create_dir_all(preferred_config_dir_path_for(home_dir.path())) + .expect("Failed to create dela config directory"); + + (project_dir, home_dir) + } + + #[test] + #[serial] + fn test_execute_allow_single_task() { + let guard = TestEnvGuard::new(); + env::set_current_dir(&guard.project_dir).expect("Failed to change directory"); + + let result = execute("test"); + assert!(result.is_ok(), "Should succeed for a single task"); + + // Verify it was added to the allowlist + let allowlist_content = + fs::read_to_string(preferred_allowlist_path_for(guard.home_dir.path())).unwrap(); + assert!(allowlist_content.contains("test")); + assert!(allowlist_content.contains("scope = \"Task\"")); + } + + #[test] + #[serial] + fn test_execute_allow_no_task() { + let guard = TestEnvGuard::new(); + env::set_current_dir(&guard.project_dir).expect("Failed to change directory"); + + let result = execute("nonexistent"); + assert!(result.is_err(), "Should fail for nonexistent task"); + assert_eq!( + result.unwrap_err().to_string(), + "dela: command or task not found: nonexistent" + ); + } + + #[test] + #[serial] + fn test_execute_allow_uninitialized() { + let guard = TestEnvGuard::new_uninitialized(); + env::set_current_dir(&guard.project_dir).expect("Failed to change directory"); + + // Create a test Makefile + let makefile_content = " +test: ## Running tests +\t@echo Testing... +"; + let mut makefile = File::create(guard.project_dir.path().join("Makefile")) + .expect("Failed to create Makefile"); + makefile + .write_all(makefile_content.as_bytes()) + .expect("Failed to write Makefile"); + + let result = execute("test"); + assert!(result.is_err(), "Should fail when dela is not initialized"); + assert_eq!( + result.unwrap_err().to_string(), + "Dela is not initialized. Please run 'dela init' first." + ); + } + + #[test] + #[serial] + fn test_execute_allow_disambiguated() { + let guard = TestEnvGuard::new(); + env::set_current_dir(&guard.project_dir).expect("Failed to change directory"); + + // Create a package.json with the same task name + let package_json_content = r#"{ + "name": "test-package", + "scripts": { + "test": "jest" + } + }"#; + + File::create(guard.project_dir.path().join("package.json")) + .unwrap() + .write_all(package_json_content.as_bytes()) + .unwrap(); + + // Create package-lock.json to ensure npm is detected + File::create(guard.project_dir.path().join("package-lock.json")) + .unwrap() + .write_all(b"{}") + .unwrap(); + + // Ambiguous task name 'test' should fail + let result = execute("test"); + assert!(result.is_err(), "Should fail for ambiguous task name"); + assert!( + result + .unwrap_err() + .to_string() + .contains("Multiple tasks named 'test' found") + ); + + // Suffixed/disambiguated task name should succeed + let result = execute("test-m"); + assert!(result.is_ok(), "Should succeed with disambiguated name"); + + let allowlist_content = + fs::read_to_string(preferred_allowlist_path_for(guard.home_dir.path())).unwrap(); + assert!(allowlist_content.contains("test")); // Note: allowlist uses original task name + assert!(allowlist_content.contains("scope = \"Task\"")); + } +} diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 00c702d..fa8758c 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -1,3 +1,4 @@ +pub mod allow; pub mod allow_command; pub mod configure_shell; pub mod get_command; diff --git a/src/main.rs b/src/main.rs index 8a809b5..5e733e9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -139,6 +139,16 @@ enum Commands { task: String, }, + /// Allow a specific task to run + /// + /// This adds the task to the allowlist at the Task scope. + /// + /// Example: dela allow build + Allow { + /// Name of the task to allow + task: String, + }, + // Internal commands (hidden from help by default) #[command(name = "configure-shell", hide = true)] ConfigureShell, @@ -183,6 +193,7 @@ async fn run_command(command: Commands) -> anyhow::Result<()> { Commands::ConfigureShell => commands::configure_shell::execute(), Commands::List { verbose, color } => commands::list::execute(verbose, &color), Commands::Run { task } => commands::run::execute(&task), + Commands::Allow { task } => commands::allow::execute(&task), Commands::GetCommand { args } => { if args.is_empty() { Err(anyhow::anyhow!("No task name provided")) diff --git a/tests/docker_noinit/test_noinit.sh b/tests/docker_noinit/test_noinit.sh index f85b72e..8490b2d 100755 --- a/tests/docker_noinit/test_noinit.sh +++ b/tests/docker_noinit/test_noinit.sh @@ -838,6 +838,56 @@ else exit 1 fi +# Test 31: Test 'dela allow' command functionality +echo "\nTest 31: Testing 'dela allow' command functionality" +echo "Initial allowlist contents:" +cat /home/testuser/.config/dela/allowlist.toml + +# Test dela allow on a single valid task +dela allow print-args >/dev/null 2>&1 || { + echo "${RED}✗ dela allow print-args failed${NC}" + exit 1 +} + +# Verify print-args was added to the allowlist with Task scope in an entry-aware way +if python3 -c ' +import sys +content = open("/home/testuser/.config/dela/allowlist.toml").read() +for entry in content.split("[[entries]]"): + if "print-args" in entry and "scope = \"Task\"" in entry: + sys.exit(0) +sys.exit(1) +'; then + echo "${GREEN}✓ print-args task was added to allowlist with Task scope via dela allow command${NC}" +else + echo "${RED}✗ print-args task was not added to allowlist with Task scope via dela allow command${NC}" + exit 1 +fi + +# Verify dela allow fails for nonexistent task +if dela allow nonexistent >/dev/null 2>&1; then + echo "${RED}✗ dela allow nonexistent succeeded but should have failed${NC}" + exit 1 +else + echo "${GREEN}✓ dela allow nonexistent failed as expected${NC}" +fi + +# We have duplicate_test.json and duplicate_test.mk still present in the directory. +# So 'test' is ambiguous. +if dela allow test >/dev/null 2>&1; then + echo "${RED}✗ dela allow test (ambiguous) succeeded but should have failed${NC}" + exit 1 +else + echo "${GREEN}✓ dela allow test (ambiguous) failed as expected${NC}" +fi + +# But dela allow test-m (disambiguated name) should succeed! +dela allow test-m >/dev/null 2>&1 || { + echo "${RED}✗ dela allow test-m (disambiguated name) failed${NC}" + exit 1 +} +echo "${GREEN}✓ dela allow test-m (disambiguated name) succeeded as expected${NC}" + # Clean up test files rm -f duplicate_test.json duplicate_test.mk list_output.txt list_output_long.txt