Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 19 additions & 11 deletions cargo_crap_baseline.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -293,7 +293,7 @@
{
"file": "./src/main.rs",
"function": "main",
"line": 198,
"line": 209,
"cyclomatic": 3.0,
"coverage": 0.0,
"crap": 12.0
Expand Down Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion dev_docs/project_plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
218 changes: 218 additions & 0 deletions src/commands/allow.rs
Original file line number Diff line number Diff line change
@@ -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(&current_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\""));
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

#[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\""));
}
}
1 change: 1 addition & 0 deletions src/commands/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
pub mod allow;
pub mod allow_command;
pub mod configure_shell;
pub mod get_command;
Expand Down
11 changes: 11 additions & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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"))
Expand Down
43 changes: 43 additions & 0 deletions tests/docker_noinit/test_noinit.sh
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,49 @@ 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
if grep -B 2 "print-args" /home/testuser/.config/dela/allowlist.toml | grep -q 'scope = "Task"'; 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}"
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
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

Expand Down