diff --git a/README.md b/README.md index 58d5e28..c03bd42 100644 --- a/README.md +++ b/README.md @@ -61,10 +61,16 @@ Or use `dela run` for subshell execution: $ dela run build ``` + +### Allowlist.toml +The allowlist is a TOML file located at `~/.config/dela/allowlist.toml`. It stores allow and deny rules at folder, file, and task level. It gets updated when you either run a task in a new folder for the first time, or when you run `dela allow ` and `dela deny ` commands explicitly. + ## MCP Server Dela includes an [MCP (Model Context Protocol)](https://modelcontextprotocol.io/) server that allows AI assistants and editors to discover and execute tasks programmatically. +The mcp executed tasks need to be already on the allowlist. The mcp server respects the allowlist, but does not give agents tools to modify it. + ### Setting Up MCP in Your Editor ```sh diff --git a/cargo_crap_baseline.json b/cargo_crap_baseline.json index c559e38..905ac6f 100644 --- a/cargo_crap_baseline.json +++ b/cargo_crap_baseline.json @@ -146,6 +146,14 @@ "coverage": 100.0, "crap": 20.0 }, + { + "file": "./src/main.rs", + "function": "main", + "line": 220, + "cyclomatic": 4.0, + "coverage": 0.0, + "crap": 20.0 + }, { "file": "./src/task_discovery/github_actions.rs", "function": "discover_github_actions_tasks", @@ -290,14 +298,6 @@ "coverage": 95.0, "crap": 12.018 }, - { - "file": "./src/main.rs", - "function": "main", - "line": 220, - "cyclomatic": 3.0, - "coverage": 0.0, - "crap": 12.0 - }, { "file": "./src/parsers/parse_makefile.rs", "function": "parse", @@ -678,6 +678,14 @@ "file": "./src/commands/allow.rs", "function": "execute", "line": 7, + "cyclomatic": 2.0, + "coverage": 0.0, + "crap": 6.0 + }, + { + "file": "./src/commands/allow.rs", + "function": "execute_inner", + "line": 12, "cyclomatic": 6.0, "coverage": 100.0, "crap": 6.0 @@ -686,6 +694,14 @@ "file": "./src/commands/deny.rs", "function": "execute", "line": 7, + "cyclomatic": 2.0, + "coverage": 0.0, + "crap": 6.0 + }, + { + "file": "./src/commands/deny.rs", + "function": "execute_inner", + "line": 12, "cyclomatic": 6.0, "coverage": 100.0, "crap": 6.0 @@ -922,6 +938,14 @@ "coverage": 100.0, "crap": 4.0 }, + { + "file": "./src/commands/mod.rs", + "function": "gate_non_interactive", + "line": 16, + "cyclomatic": 3.0, + "coverage": 75.0, + "crap": 3.140625 + }, { "file": "./src/mcp/server.rs", "function": "DelaMcpServer::send_log", diff --git a/src/commands/allow.rs b/src/commands/allow.rs index ed5d904..10ad864 100644 --- a/src/commands/allow.rs +++ b/src/commands/allow.rs @@ -5,6 +5,11 @@ use std::env; /// Executes the 'dela allow' command to add a specific task to the allowlist. pub fn execute(task_name: &str) -> anyhow::Result<()> { + super::gate_non_interactive("dela allow")?; + execute_inner(task_name) +} + +fn execute_inner(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); @@ -128,7 +133,7 @@ test: ## Running tests let guard = TestEnvGuard::new(); env::set_current_dir(&guard.project_dir).expect("Failed to change directory"); - let result = execute("test"); + let result = execute_inner("test"); assert!(result.is_ok(), "Should succeed for a single task"); // Verify it was added to the allowlist @@ -144,7 +149,7 @@ test: ## Running tests let guard = TestEnvGuard::new(); env::set_current_dir(&guard.project_dir).expect("Failed to change directory"); - let result = execute("nonexistent"); + let result = execute_inner("nonexistent"); assert!(result.is_err(), "Should fail for nonexistent task"); assert_eq!( result.unwrap_err().to_string(), @@ -169,7 +174,7 @@ test: ## Running tests .write_all(makefile_content.as_bytes()) .expect("Failed to write Makefile"); - let result = execute("test"); + let result = execute_inner("test"); assert!(result.is_err(), "Should fail when dela is not initialized"); assert_eq!( result.unwrap_err().to_string(), @@ -203,7 +208,7 @@ test: ## Running tests .unwrap(); // Ambiguous task name 'test' should fail - let result = execute("test"); + let result = execute_inner("test"); assert!(result.is_err(), "Should fail for ambiguous task name"); assert!( result @@ -213,7 +218,7 @@ test: ## Running tests ); // Suffixed/disambiguated task name should succeed - let result = execute("test-m"); + let result = execute_inner("test-m"); assert!(result.is_ok(), "Should succeed with disambiguated name"); let allowlist_content = diff --git a/src/commands/deny.rs b/src/commands/deny.rs index f47eb37..2ed6beb 100644 --- a/src/commands/deny.rs +++ b/src/commands/deny.rs @@ -5,6 +5,11 @@ 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<()> { + super::gate_non_interactive("dela deny")?; + execute_inner(task_name) +} + +fn execute_inner(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); @@ -128,7 +133,7 @@ test: ## Running tests let guard = TestEnvGuard::new(); env::set_current_dir(&guard.project_dir).expect("Failed to change directory"); - let result = execute("test"); + let result = execute_inner("test"); assert!(result.is_ok(), "Should succeed for a single task"); // Verify it was added to the allowlist with Deny scope and task name @@ -144,7 +149,7 @@ test: ## Running tests let guard = TestEnvGuard::new(); env::set_current_dir(&guard.project_dir).expect("Failed to change directory"); - let result = execute("nonexistent"); + let result = execute_inner("nonexistent"); assert!(result.is_err(), "Should fail for nonexistent task"); assert_eq!( result.unwrap_err().to_string(), @@ -169,7 +174,7 @@ test: ## Running tests .write_all(makefile_content.as_bytes()) .expect("Failed to write Makefile"); - let result = execute("test"); + let result = execute_inner("test"); assert!(result.is_err(), "Should fail when dela is not initialized"); assert_eq!( result.unwrap_err().to_string(), @@ -203,7 +208,7 @@ test: ## Running tests .unwrap(); // Ambiguous task name 'test' should fail - let result = execute("test"); + let result = execute_inner("test"); assert!(result.is_err(), "Should fail for ambiguous task name"); assert!( result @@ -213,7 +218,7 @@ test: ## Running tests ); // Suffixed/disambiguated task name should succeed - let result = execute("test-m"); + let result = execute_inner("test-m"); assert!(result.is_ok(), "Should succeed with disambiguated name"); let allowlist_content = diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 3803a98..06fd15b 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -8,3 +8,33 @@ pub mod list; pub mod mcp; pub mod run; pub mod run_command; + +use std::io::IsTerminal; + +/// Returns an error if the current session is non-interactive (no TTY). +/// This prevents scripts and agents from running `dela allow` / `dela deny`. +pub(crate) fn gate_non_interactive(command_name: &str) -> anyhow::Result<()> { + let is_terminal = std::io::stdout().is_terminal() && std::io::stdin().is_terminal(); + if !is_terminal { + anyhow::bail!( + "'{}' should only be run by human users directly, and not by scripts or agents.", + command_name + ); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_gate_non_interactive_in_test_env() { + let result = gate_non_interactive("dela allow"); + assert!(result.is_err()); + assert_eq!( + result.unwrap_err().to_string(), + "'dela allow' should only be run by human users directly, and not by scripts or agents." + ); + } +} diff --git a/src/main.rs b/src/main.rs index d9df0c2..a816697 100644 --- a/src/main.rs +++ b/src/main.rs @@ -223,13 +223,11 @@ async fn main() { let result = run_command(cli.command).await; if let Err(err) = result { - if err - .to_string() - .starts_with("dela: command or task not found") - { - eprintln!("{}", err); + let msg = err.to_string(); + if msg.starts_with("dela: command or task not found") || msg.starts_with("'dela ") { + eprintln!("{}", msg); } else { - eprintln!("Error: {}", err); + eprintln!("Error: {}", msg); } std::process::exit(1); } diff --git a/tests/docker_noinit/Dockerfile b/tests/docker_noinit/Dockerfile index 89bc142..b0e3700 100644 --- a/tests/docker_noinit/Dockerfile +++ b/tests/docker_noinit/Dockerfile @@ -16,7 +16,8 @@ RUN apk add --no-cache \ npm \ maven \ gradle \ - docker-cli && \ + docker-cli \ + util-linux && \ apk add --no-cache --repository=http://dl-cdn.alpinelinux.org/alpine/v3.21/community go-task # symlink go-task to task diff --git a/tests/docker_noinit/test_noinit.sh b/tests/docker_noinit/test_noinit.sh index 14d12ea..02d3953 100755 --- a/tests/docker_noinit/test_noinit.sh +++ b/tests/docker_noinit/test_noinit.sh @@ -844,7 +844,7 @@ 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 || { +script -eqc "dela allow print-args" /dev/null >/dev/null 2>&1 || { echo "${RED}✗ dela allow print-args failed${NC}" exit 1 } @@ -865,7 +865,7 @@ else fi # Verify dela allow fails for nonexistent task -if dela allow nonexistent >/dev/null 2>&1; then +if script -eqc "dela allow nonexistent" /dev/null >/dev/null 2>&1; then echo "${RED}✗ dela allow nonexistent succeeded but should have failed${NC}" exit 1 else @@ -874,7 +874,7 @@ 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 +if script -eqc "dela allow test" /dev/null >/dev/null 2>&1; then echo "${RED}✗ dela allow test (ambiguous) succeeded but should have failed${NC}" exit 1 else @@ -882,7 +882,7 @@ else fi # But dela allow test-m (disambiguated name) should succeed! -dela allow test-m >/dev/null 2>&1 || { +script -eqc "dela allow test-m" /dev/null >/dev/null 2>&1 || { echo "${RED}✗ dela allow test-m (disambiguated name) failed${NC}" exit 1 } @@ -892,7 +892,7 @@ echo "${GREEN}✓ dela allow test-m (disambiguated name) succeeded as expected${ echo "\nTest 32: Testing 'dela deny' command functionality" # Deny a single valid task -dela deny another-task >/dev/null 2>&1 || { +script -eqc "dela deny another-task" /dev/null >/dev/null 2>&1 || { echo "${RED}✗ dela deny another-task failed${NC}" exit 1 } @@ -944,7 +944,7 @@ else fi # Verify dela deny fails for nonexistent task -if dela deny nonexistent >/dev/null 2>&1; then +if script -eqc "dela deny nonexistent" /dev/null >/dev/null 2>&1; then echo "${RED}✗ dela deny nonexistent succeeded but should have failed${NC}" exit 1 else @@ -954,14 +954,48 @@ 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 +if script -eqc "dela deny test" /dev/null >/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 +# Test 33: Test non-interactive blocking of 'dela allow' and 'dela deny' +echo "\nTest 33: Testing non-interactive blocking of 'dela allow' and 'dela deny'" + +# Running dela allow should get blocked and print the error to stderr +exit_code=0 +output=$(dela allow print-args 2>&1) || exit_code=$? +if [ $exit_code -eq 0 ]; then + echo "${RED}✗ dela allow print-args did not fail in non-interactive session${NC}" + exit 1 +fi +if echo "$output" | grep -q "'dela allow' should only be run by human users directly, and not by scripts or agents."; then + echo "${GREEN}✓ dela allow was blocked in non-interactive session as expected${NC}" +else + echo "${RED}✗ dela allow was not blocked with the expected message${NC}" + echo "Output: $output" + exit 1 +fi + +# Running dela deny should get blocked and print the error to stderr +exit_code=0 +output=$(dela deny print-args 2>&1) || exit_code=$? +if [ $exit_code -eq 0 ]; then + echo "${RED}✗ dela deny print-args did not fail in non-interactive session${NC}" + exit 1 +fi +if echo "$output" | grep -q "'dela deny' should only be run by human users directly, and not by scripts or agents."; then + echo "${GREEN}✓ dela deny was blocked in non-interactive session as expected${NC}" +else + echo "${RED}✗ dela deny was not blocked with the expected message${NC}" + echo "Output: $output" + exit 1 +fi + # Clean up test files rm -f duplicate_test.json duplicate_test.mk list_output.txt list_output_long.txt echo "\n${GREEN}All non-init tests completed successfully!${NC}" +