diff --git a/cargo_crap_baseline.json b/cargo_crap_baseline.json index f5a36f5..c559e38 100644 --- a/cargo_crap_baseline.json +++ b/cargo_crap_baseline.json @@ -45,10 +45,10 @@ { "file": "./src/main.rs", "function": "run_command", - "line": 172, - "cyclomatic": 10.0, - "coverage": 21.428571428571427, - "crap": 58.505830903790084 + "line": 182, + "cyclomatic": 11.0, + "coverage": 20.689655172413794, + "crap": 71.36356554184265 }, { "file": "./src/prompt.rs", @@ -181,7 +181,7 @@ { "file": "./src/allowlist.rs", "function": "check_task_allowed", - "line": 144, + "line": 148, "cyclomatic": 12.0, "coverage": 66.66666666666666, "crap": 17.333333333333343 @@ -202,6 +202,14 @@ "coverage": 75.0, "crap": 17.0625 }, + { + "file": "./src/allowlist.rs", + "function": "evaluate_task_against_allowlist", + "line": 75, + "cyclomatic": 17.0, + "coverage": 100.0, + "crap": 17.0 + }, { "file": "./src/commands/allow_command.rs", "function": "execute", @@ -234,14 +242,6 @@ "coverage": 92.3076923076923, "crap": 15.10241238051889 }, - { - "file": "./src/allowlist.rs", - "function": "evaluate_task_against_allowlist", - "line": 75, - "cyclomatic": 14.0, - "coverage": 95.65217391304348, - "crap": 14.016109147694584 - }, { "file": "./src/mcp/server.rs", "function": "DelaMcpServer::task_output", @@ -293,7 +293,7 @@ { "file": "./src/main.rs", "function": "main", - "line": 209, + "line": 220, "cyclomatic": 3.0, "coverage": 0.0, "crap": 12.0 @@ -682,6 +682,14 @@ "coverage": 100.0, "crap": 6.0 }, + { + "file": "./src/commands/deny.rs", + "function": "execute", + "line": 7, + "cyclomatic": 6.0, + "coverage": 100.0, + "crap": 6.0 + }, { "file": "./src/commands/mcp.rs", "function": "Editor::config_path", @@ -757,7 +765,7 @@ { "file": "./src/allowlist.rs", "function": "is_task_allowed", - "line": 132, + "line": 136, "cyclomatic": 5.0, "coverage": 100.0, "crap": 5.0 @@ -1066,10 +1074,18 @@ "coverage": 100.0, "crap": 3.0 }, + { + "file": "./src/allowlist.rs", + "function": "allowlist_entry_for_task", + "line": 119, + "cyclomatic": 3.0, + "coverage": 100.0, + "crap": 3.0 + }, { "file": "./src/allowlist.rs", "function": "check_task_allowed_with_scope", - "line": 189, + "line": 193, "cyclomatic": 3.0, "coverage": 100.0, "crap": 3.0 @@ -1594,14 +1610,6 @@ "coverage": 100.0, "crap": 2.0 }, - { - "file": "./src/allowlist.rs", - "function": "allowlist_entry_for_task", - "line": 115, - "cyclomatic": 2.0, - "coverage": 100.0, - "crap": 2.0 - }, { "file": "./src/task_discovery.rs", "function": "discover_tasks", diff --git a/dev_docs/project_plan.md b/dev_docs/project_plan.md index b350743..53ea6d0 100644 --- a/dev_docs/project_plan.md +++ b/dev_docs/project_plan.md @@ -131,7 +131,7 @@ This plan outlines the major development phases and tasks for building `dela`, a - [x] [DTKT-32] Persist decisions in the allowlist. - [ ] [DTKT-33] Have `dela run` take an optional `--allow` flag to allow a task without prompting. - [x] [DTKT-109] Implement `dela allow` command to add allowlist entries. - - [ ] [DTKT-110] Implement `dela deny` command to add denylist entries. + - [x] [DTKT-110] Implement `dela deny` command to add denylist entries. - [ ] [DTKT-111] Implement `dela run --allow-once` command to run a command once. - [x] **Runtime Checks** diff --git a/src/allowlist.rs b/src/allowlist.rs index 37ca28c..3039e4d 100644 --- a/src/allowlist.rs +++ b/src/allowlist.rs @@ -77,10 +77,14 @@ pub fn evaluate_task_against_allowlist(task: &Task, allowlist: &Allowlist) -> Al // First pass: Check for deny entries (highest precedence) for entry in &allowlist.entries { - if let AllowScope::Deny = entry.scope - && path_matches(task_path, &entry.path, true) - { - return AllowlistMatch::Denied; + if let AllowScope::Deny = entry.scope { + if let Some(ref tasks) = entry.tasks { + if path_matches(task_path, &entry.path, false) && tasks.contains(&task.name) { + return AllowlistMatch::Denied; + } + } else if path_matches(task_path, &entry.path, true) { + return AllowlistMatch::Denied; + } } } @@ -113,7 +117,7 @@ pub fn evaluate_task_against_allowlist(task: &Task, allowlist: &Allowlist) -> Al } pub fn allowlist_entry_for_task(task: &Task, scope: AllowScope) -> AllowlistEntry { - let tasks = if scope == AllowScope::Task { + let tasks = if scope == AllowScope::Task || scope == AllowScope::Deny { Some(vec![task.name.clone()]) } else { None @@ -432,6 +436,36 @@ mod tests { reset_to_real_environment(); } + #[test] + #[serial] + fn test_is_task_allowed_deny_task_scope() { + let (temp_dir, task) = setup_test_env(); + + // Create allowlist with deny scope for specific task + let mut allowlist = Allowlist::default(); + let entry = AllowlistEntry { + path: PathBuf::from("Makefile"), + scope: AllowScope::Deny, + tasks: Some(vec!["test-task".to_string()]), + }; + allowlist.entries.push(entry); + save_allowlist(&allowlist).unwrap(); + + // Task should be denied + assert_eq!(is_task_allowed(&task).unwrap(), (false, true)); + + // A different task should NOT be denied + let other_task = create_test_task("other-task", PathBuf::from("Makefile")); + assert_eq!(is_task_allowed(&other_task).unwrap(), (false, false)); + + // A task from a different Makefile should NOT be denied + let other_path_task = create_test_task("test-task", PathBuf::from("other/Makefile")); + assert_eq!(is_task_allowed(&other_path_task).unwrap(), (false, false)); + + drop(temp_dir); + reset_to_real_environment(); + } + #[test] #[serial] fn test_is_task_allowed_uses_definition_path_for_composed_tasks() { diff --git a/src/commands/allow.rs b/src/commands/allow.rs index 00387e5..ed5d904 100644 --- a/src/commands/allow.rs +++ b/src/commands/allow.rs @@ -52,18 +52,22 @@ mod tests { struct TestEnvGuard { project_dir: TempDir, home_dir: TempDir, + original_cwd: std::path::PathBuf, } impl TestEnvGuard { fn new() -> Self { + let original_cwd = env::current_dir().unwrap_or_else(|_| std::env::temp_dir()); let (project_dir, home_dir) = setup_test_env(); Self { project_dir, home_dir, + original_cwd, } } fn new_uninitialized() -> Self { + let original_cwd = env::current_dir().unwrap_or_else(|_| std::env::temp_dir()); // 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"); @@ -74,12 +78,14 @@ mod tests { Self { project_dir: TempDir::new().expect("Failed to create temp directory"), home_dir, + original_cwd, } } } impl Drop for TestEnvGuard { fn drop(&mut self) { + let _ = env::set_current_dir(&self.original_cwd); reset_to_real_environment(); } } diff --git a/src/commands/deny.rs b/src/commands/deny.rs new file mode 100644 index 0000000..f47eb37 --- /dev/null +++ b/src/commands/deny.rs @@ -0,0 +1,224 @@ +use crate::allowlist; +use crate::task_discovery; +use crate::types::AllowScope; +use std::env; + +/// Executes the 'dela deny' command to add a specific task definition file to the denylist. +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::Deny)?; + println!( + "Denied task '{}' ({}) in 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, + original_cwd: std::path::PathBuf, + } + + impl TestEnvGuard { + fn new() -> Self { + let original_cwd = env::current_dir().unwrap_or_else(|_| std::env::temp_dir()); + let (project_dir, home_dir) = setup_test_env(); + Self { + project_dir, + home_dir, + original_cwd, + } + } + + fn new_uninitialized() -> Self { + let original_cwd = env::current_dir().unwrap_or_else(|_| std::env::temp_dir()); + // 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, + original_cwd, + } + } + } + + impl Drop for TestEnvGuard { + fn drop(&mut self) { + let _ = env::set_current_dir(&self.original_cwd); + 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_deny_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 with Deny scope and task name + let allowlist_content = + fs::read_to_string(preferred_allowlist_path_for(guard.home_dir.path())).unwrap(); + assert!(allowlist_content.contains("scope = \"Deny\"")); + assert!(allowlist_content.contains("tasks = [\"test\"]")); + } + + #[test] + #[serial] + fn test_execute_deny_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_deny_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_deny_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("scope = \"Deny\"")); + assert!(allowlist_content.contains("tasks = [\"test\"]")); + } +} diff --git a/src/commands/mod.rs b/src/commands/mod.rs index fa8758c..3803a98 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -1,6 +1,7 @@ pub mod allow; pub mod allow_command; pub mod configure_shell; +pub mod deny; pub mod get_command; pub mod init; pub mod list; diff --git a/src/main.rs b/src/main.rs index 5e733e9..d9df0c2 100644 --- a/src/main.rs +++ b/src/main.rs @@ -149,6 +149,16 @@ enum Commands { task: String, }, + /// Deny a specific task from running + /// + /// This adds the task's definition file to the allowlist at the Deny scope. + /// + /// Example: dela deny build + Deny { + /// Name of the task to deny + task: String, + }, + // Internal commands (hidden from help by default) #[command(name = "configure-shell", hide = true)] ConfigureShell, @@ -194,6 +204,7 @@ async fn run_command(command: Commands) -> anyhow::Result<()> { Commands::List { verbose, color } => commands::list::execute(verbose, &color), Commands::Run { task } => commands::run::execute(&task), Commands::Allow { task } => commands::allow::execute(&task), + Commands::Deny { task } => commands::deny::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 8490b2d..14d12ea 100755 --- a/tests/docker_noinit/test_noinit.sh +++ b/tests/docker_noinit/test_noinit.sh @@ -888,6 +888,79 @@ dela allow test-m >/dev/null 2>&1 || { } echo "${GREEN}✓ dela allow test-m (disambiguated name) succeeded as expected${NC}" +# Test 32: Test 'dela deny' command functionality +echo "\nTest 32: Testing 'dela deny' command functionality" + +# Deny a single valid task +dela deny another-task >/dev/null 2>&1 || { + echo "${RED}✗ dela deny another-task failed${NC}" + exit 1 +} + +# Verify Makefile was added to the allowlist with Deny scope and task name 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 "Makefile" in entry and "scope = \"Deny\"" in entry and "tasks = [\"another-task\"]" in entry: + sys.exit(0) +sys.exit(1) +'; then + echo "${GREEN}✓ Makefile was added to allowlist with Deny scope and tasks = [\"another-task\"] via dela deny command${NC}" +else + echo "${RED}✗ Makefile was not added to allowlist with Deny scope and tasks = [\"another-task\"] via dela deny command${NC}" + exit 1 +fi + +# Now running the denied task another-task should fail because it is denied. +# We can verify that dela allow-command on 'another-task' fails and outputs a denial message. +exit_code=0 +output=$(dela allow-command another-task 2>&1) || exit_code=$? +if [ $exit_code -eq 0 ]; then + echo "${RED}✗ Denied task another-task did not fail (exited with 0)${NC}" + exit 1 +fi +if echo "$output" | grep -q "was denied by the allowlist"; then + echo "${GREEN}✓ Denied task another-task was blocked as expected (exit code: $exit_code)${NC}" +else + echo "${RED}✗ Denied task another-task was not blocked with the expected message${NC}" + echo "Output: $output" + exit 1 +fi + +# Verify that another task from the same Makefile (like print-args) is NOT blocked as denied +exit_code_other=0 +output_other=$(dela allow-command print-args 2>&1) || exit_code_other=$? +if [ $exit_code_other -ne 0 ]; then + echo "${RED}✗ Allowed task print-args failed with exit code $exit_code_other${NC}" + echo "Output: $output_other" + exit 1 +fi +if echo "$output_other" | grep -q "was denied by the allowlist"; then + echo "${RED}✗ Other task print-args in the same Makefile was blocked as denied, but it should not be${NC}" + exit 1 +else + echo "${GREEN}✓ Other task print-args in the same Makefile was not blocked as denied (exit code: $exit_code_other)${NC}" +fi + +# Verify dela deny fails for nonexistent task +if dela deny nonexistent >/dev/null 2>&1; then + echo "${RED}✗ dela deny nonexistent succeeded but should have failed${NC}" + exit 1 +else + echo "${GREEN}✓ dela deny nonexistent failed as expected${NC}" +fi + +# Verify dela deny fails for ambiguous tasks +# We have duplicate_test.json and duplicate_test.mk still present in the directory. +# So 'test' is ambiguous. +if dela deny test >/dev/null 2>&1; then + echo "${RED}✗ dela deny test (ambiguous) succeeded but should have failed${NC}" + exit 1 +else + echo "${GREEN}✓ dela deny test (ambiguous) failed as expected${NC}" +fi + # Clean up test files rm -f duplicate_test.json duplicate_test.mk list_output.txt list_output_long.txt