feat(validate): cross-check local composite action required inputs (#78)

* feat(validate): cross-check local composite action required inputs

The validate command was happily declaring workflows "valid" even
when they used a local composite action without providing its
required inputs. The workflow would then blow up at runtime, which
is *exactly* the kind of thing a validator should catch.

The problem was that validate_action_reference() only checked
whether the local action path existed on disk. It never bothered
to read the action.yml, look at the inputs section, or verify
that required inputs were actually provided in the step's `with:`
block. So it was doing about half its job.

Thread the repo root path through the validator call chain
(evaluate_workflow_file → validate_jobs → validate_steps →
validate_action_reference) so we can resolve local action paths.
Then read and parse the action.yml, extract inputs with
`required: true` and no default, and flag any that are missing
from the step's `with:` params. Case-insensitive matching because
that's what GitHub Actions does.

Graceful degradation: if we can't find the repo root or the
action file is unreadable, we silently skip rather than blowing
up. 10 new unit tests cover the various cases.

Closes #67

* fix(validate): handle string `required` and drop unsafe cwd fallback

Two bugs in the local action input validation that just landed:

First, `find_repo_root` was falling back to `current_dir()` when no
`.git` directory was found. This is *wrong* — if you're validating a
workflow outside a git repo, the cwd could be literally anywhere, and
you'd end up resolving local action paths against some random
directory. Return `None` and skip the check instead.

Second, `required: 'true'` (as a YAML string) was silently treated as
not required, because we only checked `as_bool()`. GitHub Actions
treats the string "true" as truthy, so we should too. Add
case-insensitive string matching alongside the bool check.

While at it, add a test for the string `required` case so we don't
regress on this.
This commit is contained in:
Gokul
2026-04-02 13:49:47 +05:30
committed by GitHub
parent 452044f9d2
commit ebabe6414a
6 changed files with 520 additions and 18 deletions

1
Cargo.lock generated
View File

@@ -3563,6 +3563,7 @@ version = "0.7.3"
dependencies = [
"serde",
"serde_yaml",
"tempfile",
"wrkflw-matrix",
"wrkflw-models",
]

View File

@@ -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<ValidationRe
serde_yaml::from_str(&content).map_err(|e| format!("Invalid YAML: {}", e))?;
let mut result = ValidationResult::new();
let repo_root = find_repo_root(path);
// Check for required structure
if !workflow.is_mapping() {
@@ -28,7 +29,7 @@ pub fn evaluate_workflow_file(path: &Path, verbose: bool) -> Result<ValidationRe
// Check if jobs section exists
match workflow.get("jobs") {
Some(jobs) if jobs.is_mapping() => {
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<ValidationRe
Ok(result)
}
/// Walk up from the workflow file's directory to find the repository root (.git directory).
/// Returns `None` if no `.git` directory is found.
fn find_repo_root(workflow_path: &Path) -> Option<PathBuf> {
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
}

View File

@@ -18,3 +18,6 @@ wrkflw-matrix.workspace = true
# External dependencies
serde.workspace = true
serde_yaml.workspace = true
[dev-dependencies]
tempfile.workspace = true

View File

@@ -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<String> = 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'")));
}
}

View File

@@ -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

View File

@@ -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<String> = 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<Value> = 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<Value> = 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<Value> = 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());