diff --git a/Cargo.lock b/Cargo.lock index c2160d8..3d5a6bd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3563,6 +3563,7 @@ version = "0.7.3" dependencies = [ "serde", "serde_yaml", + "tempfile", "wrkflw-matrix", "wrkflw-models", ] diff --git a/crates/evaluator/src/lib.rs b/crates/evaluator/src/lib.rs index 0c55ca0..dbc6a51 100644 --- a/crates/evaluator/src/lib.rs +++ b/crates/evaluator/src/lib.rs @@ -1,7 +1,7 @@ use colored::*; use serde_yaml::{self, Value}; use std::fs; -use std::path::Path; +use std::path::{Path, PathBuf}; use wrkflw_models::ValidationResult; use wrkflw_validators::{validate_jobs, validate_triggers}; @@ -14,6 +14,7 @@ pub fn evaluate_workflow_file(path: &Path, verbose: bool) -> Result Result { - validate_jobs(jobs, &mut result); + validate_jobs(jobs, repo_root.as_deref(), &mut result); } Some(_) => { result.add_issue("'jobs' section is not a mapping".to_string()); @@ -58,3 +59,17 @@ pub fn evaluate_workflow_file(path: &Path, verbose: bool) -> Result Option { + let canonical = fs::canonicalize(workflow_path).ok()?; + let mut dir = canonical.parent(); + while let Some(d) = dir { + if d.join(".git").exists() { + return Some(d.to_path_buf()); + } + dir = d.parent(); + } + None +} diff --git a/crates/validators/Cargo.toml b/crates/validators/Cargo.toml index cfc2233..9e4cd84 100644 --- a/crates/validators/Cargo.toml +++ b/crates/validators/Cargo.toml @@ -18,3 +18,6 @@ wrkflw-matrix.workspace = true # External dependencies serde.workspace = true serde_yaml.workspace = true + +[dev-dependencies] +tempfile.workspace = true diff --git a/crates/validators/src/actions.rs b/crates/validators/src/actions.rs index e25777d..f10621c 100644 --- a/crates/validators/src/actions.rs +++ b/crates/validators/src/actions.rs @@ -1,9 +1,13 @@ +use std::collections::HashSet; +use std::path::Path; use wrkflw_models::ValidationResult; pub fn validate_action_reference( action_ref: &str, + with_params: Option<&serde_yaml::Mapping>, job_name: &str, step_idx: usize, + repo_root: Option<&Path>, result: &mut ValidationResult, ) { // Check if it's a local action (starts with ./) @@ -41,18 +45,487 @@ pub fn validate_action_reference( )); } - // For local actions, verify the path exists + // For local actions, validate the path and cross-check inputs if is_local_action { - let action_path = std::path::Path::new(action_ref); - if !action_path.exists() { - // We can't reliably check this during validation since the working directory - // might not be the repository root, but we'll add a warning + if let Some(root) = repo_root { + let relative_path = action_ref.strip_prefix("./").unwrap_or(action_ref); + let action_dir = root.join(relative_path); + + let action_file = { + let yml = action_dir.join("action.yml"); + let yaml = action_dir.join("action.yaml"); + if yml.exists() { + Some(yml) + } else if yaml.exists() { + Some(yaml) + } else { + None + } + }; + + match action_file { + None => { + if !action_dir.exists() { + result.add_issue(format!( + "Job '{}', step {}: Local action path '{}' does not exist", + job_name, + step_idx + 1, + action_ref + )); + } else { + result.add_issue(format!( + "Job '{}', step {}: No action.yml or action.yaml found in '{}'", + job_name, + step_idx + 1, + action_ref + )); + } + } + Some(action_file_path) => { + validate_local_action_inputs( + &action_file_path, + with_params, + action_ref, + job_name, + step_idx, + result, + ); + } + } + } + } +} + +/// Parse a local action's action.yml and validate that required inputs are provided. +fn validate_local_action_inputs( + action_file: &Path, + with_params: Option<&serde_yaml::Mapping>, + action_ref: &str, + job_name: &str, + step_idx: usize, + result: &mut ValidationResult, +) { + let content = match std::fs::read_to_string(action_file) { + Ok(c) => c, + Err(_) => return, + }; + + let action_def: serde_yaml::Value = match serde_yaml::from_str(&content) { + Ok(v) => v, + Err(_) => { result.add_issue(format!( - "Job '{}', step {}: Local action path '{}' may not exist at runtime", + "Job '{}', step {}: Failed to parse action definition at '{}'", job_name, step_idx + 1, action_ref )); + return; + } + }; + + let inputs_map = match action_def.get("inputs").and_then(|v| v.as_mapping()) { + Some(m) => m, + None => return, + }; + + let provided_keys: HashSet = with_params + .map(|m| { + m.keys() + .filter_map(|k| k.as_str()) + .map(|s| s.to_lowercase()) + .collect() + }) + .unwrap_or_default(); + + for (input_name, input_def) in inputs_map { + let input_name_str = match input_name.as_str() { + Some(s) => s, + None => continue, + }; + + let is_required = input_def + .get("required") + .map(|v| match v { + serde_yaml::Value::Bool(b) => *b, + serde_yaml::Value::String(s) => s.eq_ignore_ascii_case("true"), + _ => false, + }) + .unwrap_or(false); + + let has_default = input_def.get("default").is_some(); + + if is_required && !has_default && !provided_keys.contains(&input_name_str.to_lowercase()) { + result.add_issue(format!( + "Job '{}', step {}: Local action '{}' requires input '{}' but it is not provided in 'with'", + job_name, + step_idx + 1, + action_ref, + input_name_str + )); } } } + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + use tempfile::tempdir; + + #[test] + fn test_missing_required_input() { + let dir = tempdir().unwrap(); + let action_dir = dir.path().join("my-action"); + fs::create_dir(&action_dir).unwrap(); + fs::write( + action_dir.join("action.yml"), + r#" +name: 'Test Action' +inputs: + token: + description: 'A required token' + required: true +runs: + using: 'composite' + steps: + - run: echo hello +"#, + ) + .unwrap(); + + let mut result = ValidationResult::new(); + validate_action_reference( + "./my-action", + None, + "build", + 0, + Some(dir.path()), + &mut result, + ); + + assert!(!result.is_valid); + assert!(result + .issues + .iter() + .any(|i| i.contains("requires input 'token'"))); + } + + #[test] + fn test_required_input_provided() { + let dir = tempdir().unwrap(); + let action_dir = dir.path().join("my-action"); + fs::create_dir(&action_dir).unwrap(); + fs::write( + action_dir.join("action.yml"), + r#" +name: 'Test Action' +inputs: + token: + description: 'A required token' + required: true +runs: + using: 'composite' + steps: + - run: echo hello +"#, + ) + .unwrap(); + + let mut with = serde_yaml::Mapping::new(); + with.insert( + serde_yaml::Value::String("token".to_string()), + serde_yaml::Value::String("my-secret".to_string()), + ); + + let mut result = ValidationResult::new(); + validate_action_reference( + "./my-action", + Some(&with), + "build", + 0, + Some(dir.path()), + &mut result, + ); + + assert!(result.is_valid); + assert!(result.issues.is_empty()); + } + + #[test] + fn test_required_input_with_default_passes() { + let dir = tempdir().unwrap(); + let action_dir = dir.path().join("my-action"); + fs::create_dir(&action_dir).unwrap(); + fs::write( + action_dir.join("action.yml"), + r#" +name: 'Test Action' +inputs: + token: + description: 'Has a default' + required: true + default: 'fallback' +runs: + using: 'composite' + steps: + - run: echo hello +"#, + ) + .unwrap(); + + let mut result = ValidationResult::new(); + validate_action_reference( + "./my-action", + None, + "build", + 0, + Some(dir.path()), + &mut result, + ); + + assert!(result.is_valid); + } + + #[test] + fn test_no_inputs_section_passes() { + let dir = tempdir().unwrap(); + let action_dir = dir.path().join("my-action"); + fs::create_dir(&action_dir).unwrap(); + fs::write( + action_dir.join("action.yml"), + r#" +name: 'Test Action' +runs: + using: 'composite' + steps: + - run: echo hello +"#, + ) + .unwrap(); + + let mut result = ValidationResult::new(); + validate_action_reference( + "./my-action", + None, + "build", + 0, + Some(dir.path()), + &mut result, + ); + + assert!(result.is_valid); + } + + #[test] + fn test_action_dir_does_not_exist() { + let dir = tempdir().unwrap(); + + let mut result = ValidationResult::new(); + validate_action_reference( + "./nonexistent", + None, + "build", + 0, + Some(dir.path()), + &mut result, + ); + + assert!(!result.is_valid); + assert!(result.issues.iter().any(|i| i.contains("does not exist"))); + } + + #[test] + fn test_action_dir_exists_but_no_action_yml() { + let dir = tempdir().unwrap(); + let action_dir = dir.path().join("my-action"); + fs::create_dir(&action_dir).unwrap(); + + let mut result = ValidationResult::new(); + validate_action_reference( + "./my-action", + None, + "build", + 0, + Some(dir.path()), + &mut result, + ); + + assert!(!result.is_valid); + assert!(result + .issues + .iter() + .any(|i| i.contains("No action.yml or action.yaml"))); + } + + #[test] + fn test_case_insensitive_input_matching() { + let dir = tempdir().unwrap(); + let action_dir = dir.path().join("my-action"); + fs::create_dir(&action_dir).unwrap(); + fs::write( + action_dir.join("action.yml"), + r#" +name: 'Test Action' +inputs: + My-Token: + description: 'Mixed case' + required: true +runs: + using: 'composite' + steps: + - run: echo hello +"#, + ) + .unwrap(); + + let mut with = serde_yaml::Mapping::new(); + with.insert( + serde_yaml::Value::String("my-token".to_string()), + serde_yaml::Value::String("value".to_string()), + ); + + let mut result = ValidationResult::new(); + validate_action_reference( + "./my-action", + Some(&with), + "build", + 0, + Some(dir.path()), + &mut result, + ); + + assert!(result.is_valid); + } + + #[test] + fn test_multiple_missing_required_inputs() { + let dir = tempdir().unwrap(); + let action_dir = dir.path().join("my-action"); + fs::create_dir(&action_dir).unwrap(); + fs::write( + action_dir.join("action.yml"), + r#" +name: 'Test Action' +inputs: + token: + description: 'Required' + required: true + name: + description: 'Also required' + required: true + optional: + description: 'Not required' + required: false +runs: + using: 'composite' + steps: + - run: echo hello +"#, + ) + .unwrap(); + + let mut result = ValidationResult::new(); + validate_action_reference( + "./my-action", + None, + "build", + 0, + Some(dir.path()), + &mut result, + ); + + assert!(!result.is_valid); + assert_eq!( + result + .issues + .iter() + .filter(|i| i.contains("requires input")) + .count(), + 2 + ); + } + + #[test] + fn test_required_as_string_true() { + let dir = tempdir().unwrap(); + let action_dir = dir.path().join("my-action"); + fs::create_dir(&action_dir).unwrap(); + fs::write( + action_dir.join("action.yml"), + r#" +name: 'Test Action' +inputs: + token: + description: 'Required as string' + required: 'true' +runs: + using: 'composite' + steps: + - run: echo hello +"#, + ) + .unwrap(); + + let mut result = ValidationResult::new(); + validate_action_reference( + "./my-action", + None, + "build", + 0, + Some(dir.path()), + &mut result, + ); + + assert!(!result.is_valid); + assert!(result + .issues + .iter() + .any(|i| i.contains("requires input 'token'"))); + } + + #[test] + fn test_repo_root_none_skips_validation() { + let mut result = ValidationResult::new(); + validate_action_reference("./my-action", None, "build", 0, None, &mut result); + + assert!(result.is_valid); + } + + #[test] + fn test_action_yaml_extension() { + let dir = tempdir().unwrap(); + let action_dir = dir.path().join("my-action"); + fs::create_dir(&action_dir).unwrap(); + fs::write( + action_dir.join("action.yaml"), + r#" +name: 'Test Action' +inputs: + token: + description: 'Required' + required: true +runs: + using: 'composite' + steps: + - run: echo hello +"#, + ) + .unwrap(); + + let mut result = ValidationResult::new(); + validate_action_reference( + "./my-action", + None, + "build", + 0, + Some(dir.path()), + &mut result, + ); + + assert!(!result.is_valid); + assert!(result + .issues + .iter() + .any(|i| i.contains("requires input 'token'"))); + } +} diff --git a/crates/validators/src/jobs.rs b/crates/validators/src/jobs.rs index e793d45..29bf6f8 100644 --- a/crates/validators/src/jobs.rs +++ b/crates/validators/src/jobs.rs @@ -1,10 +1,11 @@ use std::collections::{HashMap, HashSet}; +use std::path::Path; use crate::{validate_matrix, validate_steps}; use serde_yaml::Value; use wrkflw_models::ValidationResult; -pub fn validate_jobs(jobs: &Value, result: &mut ValidationResult) { +pub fn validate_jobs(jobs: &Value, repo_root: Option<&Path>, result: &mut ValidationResult) { if let Value::Mapping(jobs_map) = jobs { if jobs_map.is_empty() { result.add_issue("'jobs' section is empty".to_string()); @@ -35,7 +36,7 @@ pub fn validate_jobs(jobs: &Value, result: &mut ValidationResult) { job_name )); } else { - validate_steps(steps, job_name, result); + validate_steps(steps, job_name, repo_root, result); } } Some(_) => { @@ -254,7 +255,7 @@ job-c: "#; let jobs: Value = serde_yaml::from_str(yaml).unwrap(); let mut result = ValidationResult::new(); - validate_jobs(&jobs, &mut result); + validate_jobs(&jobs, None, &mut result); assert!( result @@ -296,7 +297,7 @@ job-e: "#; let jobs: Value = serde_yaml::from_str(yaml).unwrap(); let mut result = ValidationResult::new(); - validate_jobs(&jobs, &mut result); + validate_jobs(&jobs, None, &mut result); let cycle_issues: Vec<_> = result .issues @@ -340,7 +341,7 @@ deploy: "#; let jobs: Value = serde_yaml::from_str(yaml).unwrap(); let mut result = ValidationResult::new(); - validate_jobs(&jobs, &mut result); + validate_jobs(&jobs, None, &mut result); assert!( !result diff --git a/crates/validators/src/steps.rs b/crates/validators/src/steps.rs index df43917..45d7635 100644 --- a/crates/validators/src/steps.rs +++ b/crates/validators/src/steps.rs @@ -1,9 +1,15 @@ use crate::validate_action_reference; use serde_yaml::Value; use std::collections::HashSet; +use std::path::Path; use wrkflw_models::ValidationResult; -pub fn validate_steps(steps: &[Value], job_name: &str, result: &mut ValidationResult) { +pub fn validate_steps( + steps: &[Value], + job_name: &str, + repo_root: Option<&Path>, + result: &mut ValidationResult, +) { let mut step_ids: HashSet = HashSet::new(); for (i, step) in steps.iter().enumerate() { @@ -44,7 +50,10 @@ pub fn validate_steps(steps: &[Value], job_name: &str, result: &mut ValidationRe // Validate action reference if 'uses' is present if let Some(Value::String(uses)) = step_map.get(Value::String("uses".to_string())) { - validate_action_reference(uses, job_name, i, result); + let with_params = step_map + .get(Value::String("with".to_string())) + .and_then(|v| v.as_mapping()); + validate_action_reference(uses, with_params, job_name, i, repo_root, result); } } else { result.add_issue(format!( @@ -68,7 +77,7 @@ mod tests { "#; let steps: Vec = serde_yaml::from_str(yaml).unwrap(); let mut result = ValidationResult::new(); - validate_steps(&steps, "test-job", &mut result); + validate_steps(&steps, "test-job", None, &mut result); assert!(!result.is_valid); assert!(result @@ -85,7 +94,7 @@ mod tests { "#; let steps: Vec = serde_yaml::from_str(yaml).unwrap(); let mut result = ValidationResult::new(); - validate_steps(&steps, "test-job", &mut result); + validate_steps(&steps, "test-job", None, &mut result); assert!(result.is_valid); assert!(result.issues.is_empty()); @@ -99,7 +108,7 @@ mod tests { "#; let steps: Vec = serde_yaml::from_str(yaml).unwrap(); let mut result = ValidationResult::new(); - validate_steps(&steps, "test-job", &mut result); + validate_steps(&steps, "test-job", None, &mut result); assert!(result.is_valid); assert!(result.issues.is_empty());