testing_conventions/
workflow_lint.rs

1//! `workflow-lint` — flag a program encoded in GitHub Actions YAML.
2//!
3//! A `run:` or `actions/github-script` body should be wiring: a few straight-line commands, or a
4//! lone guard around an early exit. Iteration, multi-branch dispatch, and text-munging make it a
5//! program, and a program in YAML is untested, un-runnable locally, and drifts in silence. It
6//! belongs in a tested package in the repository's own language, invoked as a one-line `run:`.
7//!
8//! This is a pragmatic scanner, not a shell parser: it flags the high-signal markers of "this is a
9//! program" and tolerates straight-line glue. It favors precision over recall — a borderline body
10//! that slips through is still worth extracting, because an extracted script is testable.
11
12use std::path::{Path, PathBuf};
13
14use anyhow::{Context, Result};
15
16/// Straight-line bodies longer than this must move out, branch-free or not: past a dozen commands
17/// the step is a script whatever its control flow.
18pub const MAX_GLUE_LINES: usize = 12;
19
20/// Shell keywords that, at a command position, mean iteration or multi-branch dispatch.
21const LOGIC_KEYWORDS: [(&str, &str); 5] = [
22    ("for", "for loop"),
23    ("while", "while loop"),
24    ("until", "until loop"),
25    ("select", "select loop"),
26    ("case", "case dispatch"),
27];
28
29/// Commands whose whole purpose is rewriting text, which is a transformation worth testing.
30const MUNGING_COMMANDS: [&str; 2] = ["awk", "sed"];
31
32/// One step whose body encodes logic.
33#[derive(Debug, Clone, PartialEq, Eq)]
34pub struct Finding {
35    pub file: PathBuf,
36    /// 1-based line of the body the reasons were found in.
37    pub line: usize,
38    /// The step's `name`, else its `uses`, else a placeholder.
39    pub step: String,
40    /// Which kind of body: `run` or `github-script`.
41    pub kind: &'static str,
42    /// Why the body is too complex to stay inline, in the order found.
43    pub reasons: Vec<String>,
44}
45
46/// Why `body` is too complex to live inline. Empty means it reads as wiring.
47pub fn flag_reasons(body: &str) -> Vec<String> {
48    let mut reasons: Vec<String> = Vec::new();
49    for (keyword, name) in LOGIC_KEYWORDS {
50        if body.lines().any(|line| starts_command(line, keyword)) {
51            reasons.push(name.to_string());
52        }
53    }
54    for command in MUNGING_COMMANDS {
55        if body.lines().any(|line| runs_command(line, command)) {
56            reasons.push(command.to_string());
57        }
58    }
59    let count = significant_lines(body);
60    if count > MAX_GLUE_LINES {
61        reasons.push(format!("{count} lines (> {MAX_GLUE_LINES})"));
62    }
63    reasons
64}
65
66/// How many lines of `body` carry a command — blanks, comments, and the `set -…` prologue are
67/// bookkeeping, not length.
68fn significant_lines(body: &str) -> usize {
69    body.lines()
70        .map(str::trim)
71        .filter(|line| !line.is_empty() && !line.starts_with('#') && !line.starts_with("set -"))
72        .count()
73}
74
75/// `true` when `line`'s first word is `keyword`. Matching the whole first word is what keeps
76/// `before_hook` off the `for` marker and `casements=3` off the `case` one.
77fn starts_command(line: &str, keyword: &str) -> bool {
78    line.split_whitespace().next() == Some(keyword)
79}
80
81/// `true` when `command` runs anywhere in `line` — at the start, or after a pipe, a separator, or a
82/// substitution opener. `echo x | awk …` is as much a transformation as a bare `awk`.
83fn runs_command(line: &str, command: &str) -> bool {
84    command_segments(line).any(|segment| segment.split_whitespace().next() == Some(command))
85}
86
87/// `line` split at the shell positions a fresh command can start after: `|`, `;`, `&`, and the
88/// `$(` / `` ` `` substitution openers. Splitting on the single characters covers `||` and `&&`
89/// too, since the extra empty segment matches nothing.
90fn command_segments(line: &str) -> impl Iterator<Item = &str> {
91    line.split(['|', ';', '&', '(', ')', '`', '{', '}'])
92}
93
94/// Every logic-bearing step in one workflow or composite-action document.
95///
96/// A document that does not parse yields no findings rather than an error: `workflow-lint` judges
97/// the shape of a body it can read, and unparseable YAML is [actionlint]'s to report, with far
98/// better messages than this check could give.
99///
100/// [actionlint]: https://github.com/rhysd/actionlint
101pub fn find_violations(yaml_text: &str, file: impl AsRef<Path>) -> Vec<Finding> {
102    let file = file.as_ref();
103    let root = match marked_yaml::parse_yaml(0, yaml_text) {
104        Ok(marked_yaml::types::Node::Mapping(root)) => root,
105        _ => return Vec::new(),
106    };
107
108    let mut findings = Vec::new();
109    for step in steps(&root) {
110        let label = step_label(step);
111        for (kind, body, line) in bodies(step) {
112            let reasons = flag_reasons(&body);
113            if !reasons.is_empty() {
114                findings.push(Finding {
115                    file: file.to_path_buf(),
116                    line,
117                    step: label.clone(),
118                    kind,
119                    reasons,
120                });
121            }
122        }
123    }
124    findings
125}
126
127/// Every step mapping in a document: each job's `steps` for a workflow, `runs.steps` for a
128/// composite action.
129fn steps(
130    root: &marked_yaml::types::MarkedMappingNode,
131) -> Vec<&marked_yaml::types::MarkedMappingNode> {
132    let mut out = Vec::new();
133    if let Some(jobs) = root.get_mapping("jobs") {
134        for (_, job) in jobs.iter() {
135            if let Some(job) = job.as_mapping() {
136                push_steps(job, &mut out);
137            }
138        }
139    }
140    if let Some(runs) = root.get_mapping("runs") {
141        push_steps(runs, &mut out);
142    }
143    out
144}
145
146/// Append `owner`'s `steps` sequence, when it has one, to `out`.
147fn push_steps<'a>(
148    owner: &'a marked_yaml::types::MarkedMappingNode,
149    out: &mut Vec<&'a marked_yaml::types::MarkedMappingNode>,
150) {
151    let Some(steps) = owner.get_sequence("steps") else {
152        return;
153    };
154    out.extend(steps.iter().filter_map(|step| step.as_mapping()));
155}
156
157/// How the step names itself in a report: its `name`, else the action it `uses`.
158fn step_label(step: &marked_yaml::types::MarkedMappingNode) -> String {
159    step.get_scalar("name")
160        .or_else(|| step.get_scalar("uses"))
161        .map(|node| node.as_str().to_string())
162        .unwrap_or_else(|| "<unnamed step>".to_string())
163}
164
165/// Each `(kind, body, line)` a step carries: its `run`, and the `script` of a `github-script` step.
166fn bodies(step: &marked_yaml::types::MarkedMappingNode) -> Vec<(&'static str, String, usize)> {
167    let mut out = Vec::new();
168    if let Some(run) = step.get_scalar("run") {
169        out.push(("run", run.as_str().to_string(), line_of(run.span())));
170    }
171    out.extend(github_script_body(step));
172    out
173}
174
175/// The `script` body a `github-script` step carries. A step using any other action, or one
176/// passing no `script`, carries none.
177fn github_script_body(
178    step: &marked_yaml::types::MarkedMappingNode,
179) -> Option<(&'static str, String, usize)> {
180    let uses = step.get_scalar("uses")?;
181    if !uses.as_str().starts_with("actions/github-script") {
182        return None;
183    }
184    let script = step.get_mapping("with")?.get_scalar("script")?;
185    Some((
186        "github-script",
187        script.as_str().to_string(),
188        line_of(script.span()),
189    ))
190}
191
192/// The 1-based line a node starts on, or 0 when the parser recorded no position.
193fn line_of(span: &marked_yaml::Span) -> usize {
194    span.start().map(|marker| marker.line()).unwrap_or(0)
195}
196
197/// Every logic-bearing step under `path` — a workflow file, or a directory to search — in
198/// file-then-line order.
199///
200/// A `path` that is not there yields no findings: a repository with no CI has nothing to judge, so
201/// the default `.github` must not be an error.
202pub fn scan(path: impl AsRef<Path>) -> Result<Vec<Finding>> {
203    let path = path.as_ref();
204    let mut files = Vec::new();
205    if path.exists() {
206        collect_ci_files(path, &mut files)?;
207    }
208    files.sort();
209
210    let mut findings = Vec::new();
211    for file in files {
212        let text = std::fs::read_to_string(&file)
213            .with_context(|| format!("reading workflow `{}`", file.display()))?;
214        findings.extend(find_violations(&text, &file));
215    }
216    Ok(findings)
217}
218
219/// `true` when `<base>...HEAD` in `repo` changes a file [`scan`] reads under `path`, which is
220/// relative to `repo`. Without a base the whole tree is in scope.
221pub fn in_scope(repo: &Path, path: &Path, base: Option<&str>) -> Result<bool> {
222    let Some(base) = base else {
223        return Ok(true);
224    };
225    let changed = changed_under(repo, path, base)?;
226    let named_file = repo.join(path).is_file();
227    Ok(changed
228        .iter()
229        .any(|file| named_file || is_ci_file(Path::new(file))))
230}
231
232/// `--no-renames` lists a moved workflow at both ends, so a move out of `workflows/` still counts.
233const DIFF_ARGS: [&str; 8] = [
234    "-c",
235    "core.quotepath=off",
236    "--literal-pathspecs",
237    "diff",
238    "--name-only",
239    "--no-ext-diff",
240    "--no-renames",
241    "--relative",
242];
243
244/// Every path under `path` that `<base>...HEAD` touched, relative to `repo`.
245fn changed_under(repo: &Path, path: &Path, base: &str) -> Result<Vec<String>> {
246    let range = format!("{base}...HEAD");
247    let output = std::process::Command::new("git")
248        .current_dir(repo)
249        .args(DIFF_ARGS)
250        .args([range.as_str(), "--"])
251        .arg(path)
252        .output()
253        .with_context(|| format!("running `git diff` in `{}`", repo.display()))?;
254    if !output.status.success() {
255        let stderr = String::from_utf8_lossy(&output.stderr);
256        anyhow::bail!("`git diff {range}` failed: {}", stderr.trim());
257    }
258    Ok(diff_paths(&String::from_utf8_lossy(&output.stdout)))
259}
260
261/// The paths a `git diff --name-only` printed, C-quoting undone.
262fn diff_paths(stdout: &str) -> Vec<String> {
263    stdout
264        .lines()
265        .filter(|line| !line.is_empty())
266        .map(|line| crate::git_path::unquote_c_path(line).replace('\\', "/"))
267        .collect()
268}
269
270/// Collect the YAML GitHub actually executes under `path` into `out`: `path` itself when it is a
271/// file, else every qualifying file beneath it, recursively. A named file is always scanned, so a
272/// caller can point the check anywhere.
273fn collect_ci_files(path: &Path, out: &mut Vec<PathBuf>) -> Result<()> {
274    if path.is_file() {
275        out.push(path.to_path_buf());
276        return Ok(());
277    }
278    let entries = std::fs::read_dir(path)
279        .with_context(|| format!("reading directory `{}`", path.display()))?;
280    for entry in entries {
281        let child = crate::walk::dir_entry(entry, path)?.path();
282        if child.is_dir() {
283            collect_ci_files(&child, out)?;
284        } else if is_ci_file(&child) {
285            out.push(child);
286        }
287    }
288    Ok(())
289}
290
291/// `true` when GitHub reads `path` as a workflow or a composite action: a YAML file directly inside
292/// a `workflows` directory, or one named `action.yml` / `action.yaml` anywhere.
293///
294/// Discovery mirrors GitHub's own rules rather than taking every `*.yml` in the tree, so a fixture
295/// or a lockfile that happens to sit under `.github` is never mistaken for CI.
296fn is_ci_file(path: &Path) -> bool {
297    if !matches!(
298        path.extension().and_then(|e| e.to_str()),
299        Some("yml" | "yaml")
300    ) {
301        return false;
302    }
303    let name = path.file_name().and_then(|n| n.to_str());
304    if matches!(name, Some("action.yml" | "action.yaml")) {
305        return true;
306    }
307    path.parent()
308        .and_then(Path::file_name)
309        .and_then(|n| n.to_str())
310        == Some("workflows")
311}
312
313#[cfg(test)]
314mod tests {
315    use super::*;
316
317    #[test]
318    fn changed_under_reports_a_spawn_failure() {
319        let nowhere = Path::new("/nonexistent-tc-workflow-lint");
320        let err = changed_under(nowhere, Path::new(".github"), "main").unwrap_err();
321        assert!(format!("{err:#}").contains("running `git diff`"), "{err:#}");
322    }
323
324    #[test]
325    fn diff_paths_undoes_c_quoting_and_drops_blank_lines() {
326        assert_eq!(
327            diff_paths("\".github/workflows/a\\\"b.yml\"\n\n.github/action.yml\n"),
328            vec![".github/workflows/a\"b.yml", ".github/action.yml"]
329        );
330    }
331
332    #[test]
333    fn straight_line_glue_is_not_logic() {
334        assert_eq!(
335            flag_reasons(
336                "pkg=@x/linux\nmkdir -p \"$pkg/bin\"\ninstall -m 755 bin/x \"$pkg/bin/x\""
337            ),
338            Vec::<String>::new()
339        );
340    }
341
342    #[test]
343    fn a_lone_guard_around_an_exit_is_glue() {
344        let body = "set -euo pipefail\n\
345                    private=$(gh api repos/x --jq .private)\n\
346                    if [ \"$private\" = \"true\" ]; then\n\
347                      echo \"::error::private\"\n\
348                      exit 1\n\
349                    fi\n\
350                    echo ok\n";
351        assert_eq!(flag_reasons(body), Vec::<String>::new());
352    }
353
354    #[test]
355    fn each_loop_and_dispatch_keyword_is_named() {
356        assert_eq!(
357            flag_reasons("for name in $NAMES; do\n  npm publish \"$name\"\ndone"),
358            vec!["for loop"]
359        );
360        assert_eq!(
361            flag_reasons("while read -r l; do\n  echo $l\ndone"),
362            vec!["while loop"]
363        );
364        assert_eq!(
365            flag_reasons("until ok; do\n  sleep 1\ndone"),
366            vec!["until loop"]
367        );
368        assert_eq!(
369            flag_reasons("select x in a b; do\n  echo $x\ndone"),
370            vec!["select loop"]
371        );
372        assert_eq!(
373            flag_reasons("case \"$1\" in\n  a) echo a ;;\nesac"),
374            vec!["case dispatch"]
375        );
376    }
377
378    #[test]
379    fn a_keyword_is_only_a_marker_at_a_command_position() {
380        // `before` / `format` / `casements` share a prefix with a keyword and must not trip it.
381        assert_eq!(
382            flag_reasons("before_hook\nformat_output\ncasements=3"),
383            Vec::<String>::new()
384        );
385        // A keyword as an argument is not a command either.
386        assert_eq!(flag_reasons("echo for while case"), Vec::<String>::new());
387    }
388
389    #[test]
390    fn text_munging_counts_wherever_the_command_runs() {
391        assert_eq!(flag_reasons("awk -F/ '{print $2}' f"), vec!["awk"]);
392        assert_eq!(flag_reasons("echo x | awk -F/ '{print $2}'"), vec!["awk"]);
393        assert_eq!(flag_reasons("cat f | sed 's/a/b/'"), vec!["sed"]);
394        assert_eq!(flag_reasons("v=$(sed -n 1p f)"), vec!["sed"]);
395        assert_eq!(flag_reasons("a; sed -i s/x/y/ f"), vec!["sed"]);
396        assert_eq!(flag_reasons("ok && awk 'END{print NR}' f"), vec!["awk"]);
397    }
398
399    #[test]
400    fn a_munging_command_named_as_an_argument_is_not_running() {
401        assert_eq!(
402            flag_reasons("echo 'use awk for this'"),
403            Vec::<String>::new()
404        );
405        assert_eq!(flag_reasons("apt-get install -y sed"), Vec::<String>::new());
406    }
407
408    #[test]
409    fn a_long_straight_line_body_is_a_script_by_length() {
410        let body = (0..MAX_GLUE_LINES + 3)
411            .map(|i| format!("cmd{i}"))
412            .collect::<Vec<_>>()
413            .join("\n");
414        assert_eq!(flag_reasons(&body), vec!["15 lines (> 12)"]);
415    }
416
417    #[test]
418    fn the_length_count_ignores_bookkeeping() {
419        // Exactly at the ceiling passes, and the prologue, comments, and blanks do not count.
420        let body = (0..MAX_GLUE_LINES)
421            .map(|i| format!("cmd{i}"))
422            .collect::<Vec<_>>()
423            .join("\n");
424        assert_eq!(flag_reasons(&body), Vec::<String>::new());
425        assert_eq!(
426            flag_reasons("set -euo pipefail\n# a note\n\necho hi"),
427            Vec::<String>::new()
428        );
429    }
430
431    #[test]
432    fn every_reason_a_body_earns_is_reported() {
433        let body = "for f in *; do\n  case $f in\n    a) sed -i s/x/y/ $f ;;\n  esac\ndone";
434        assert_eq!(flag_reasons(body), vec!["for loop", "case dispatch", "sed"]);
435    }
436
437    const LOOP_WORKFLOW: &str = "jobs:\n  publish:\n    steps:\n      - name: Publish stubs\n        run: |\n          for name in $NAMES; do\n            npm publish \"$name\"\n          done\n";
438
439    #[test]
440    fn a_run_step_reports_its_file_name_and_line() {
441        let findings = find_violations(LOOP_WORKFLOW, "w.yml");
442        assert_eq!(findings.len(), 1);
443        assert_eq!(findings[0].step, "Publish stubs");
444        assert_eq!(findings[0].kind, "run");
445        assert_eq!(findings[0].reasons, vec!["for loop"]);
446        assert_eq!(findings[0].file, PathBuf::from("w.yml"));
447        assert_eq!(findings[0].line, 6, "the body starts on line 6");
448    }
449
450    #[test]
451    fn a_github_script_body_is_judged_too() {
452        let yaml = "jobs:\n  label:\n    steps:\n      - uses: actions/github-script@v7\n        with:\n          script: |\n            for (const i of items) {\n              core.info(i)\n            }\n";
453        let findings = find_violations(yaml, "w.yml");
454        assert_eq!(findings.len(), 1);
455        assert_eq!(findings[0].kind, "github-script");
456        assert_eq!(findings[0].step, "actions/github-script@v7");
457        assert_eq!(findings[0].reasons, vec!["for loop"]);
458    }
459
460    #[test]
461    fn another_actions_script_input_is_not_a_github_script_body() {
462        let yaml = "jobs:\n  j:\n    steps:\n      - uses: some/other@v1\n        with:\n          script: |\n            for x in 1 2; do echo $x; done\n";
463        assert_eq!(find_violations(yaml, "w.yml"), Vec::new());
464    }
465
466    #[test]
467    fn a_composite_action_is_scanned_through_runs_steps() {
468        let yaml = "runs:\n  using: composite\n  steps:\n    - name: Munge\n      shell: bash\n      run: cat f | sed 's/a/b/'\n";
469        let findings = find_violations(yaml, "action.yml");
470        assert_eq!(findings.len(), 1);
471        assert_eq!(findings[0].step, "Munge");
472        assert_eq!(findings[0].reasons, vec!["sed"]);
473    }
474
475    #[test]
476    fn a_clean_workflow_yields_nothing() {
477        let yaml = "jobs:\n  build:\n    steps:\n      - uses: actions/checkout@v4\n      - run: pnpm install --frozen-lockfile\n      - name: Test\n        run: pnpm test\n";
478        assert_eq!(find_violations(yaml, "w.yml"), Vec::new());
479    }
480
481    #[test]
482    fn a_step_without_a_name_falls_back_to_its_uses_then_a_placeholder() {
483        let named_by_uses = "jobs:\n  j:\n    steps:\n      - uses: actions/setup-node@v4\n        run: for x in 1; do echo $x; done\n";
484        assert_eq!(
485            find_violations(named_by_uses, "w.yml")[0].step,
486            "actions/setup-node@v4"
487        );
488
489        let unnamed = "jobs:\n  j:\n    steps:\n      - run: for x in 1; do echo $x; done\n";
490        assert_eq!(find_violations(unnamed, "w.yml")[0].step, "<unnamed step>");
491    }
492
493    #[test]
494    fn unparseable_yaml_is_actionlints_to_report_not_ours() {
495        assert_eq!(find_violations("jobs: [unclosed\n", "w.yml"), Vec::new());
496    }
497
498    #[test]
499    fn a_document_with_no_steps_anywhere_yields_nothing() {
500        assert_eq!(find_violations("name: CI\non: push\n", "w.yml"), Vec::new());
501        assert_eq!(find_violations("- a\n- b\n", "w.yml"), Vec::new());
502        assert_eq!(find_violations("", "w.yml"), Vec::new());
503        // A job, or a composite `runs:`, that declares no steps at all.
504        assert_eq!(
505            find_violations("jobs:\n  a:\n    runs-on: ubuntu-latest\n", "w.yml"),
506            Vec::new()
507        );
508        assert_eq!(
509            find_violations("runs:\n  using: node20\n  main: index.js\n", "action.yml"),
510            Vec::new()
511        );
512    }
513
514    #[test]
515    fn a_job_that_is_not_a_mapping_is_skipped() {
516        // `jobs:` holding a scalar is nonsense GitHub would reject; it must not panic here.
517        assert_eq!(find_violations("jobs:\n  a: 3\n", "w.yml"), Vec::new());
518    }
519
520    #[test]
521    fn a_github_script_step_carrying_no_script_has_no_body() {
522        let yaml = "jobs:\n  j:\n    steps:\n      - uses: actions/github-script@v7\n";
523        assert_eq!(find_violations(yaml, "w.yml"), Vec::new());
524    }
525
526    #[test]
527    fn discovery_follows_githubs_own_rules() {
528        assert!(is_ci_file(Path::new(".github/workflows/ci.yml")));
529        assert!(is_ci_file(Path::new(".github/workflows/ci.yaml")));
530        assert!(is_ci_file(Path::new(".github/actions/detect/action.yml")));
531        assert!(is_ci_file(Path::new("tools/thing/action.yaml")));
532
533        // Not CI: a lockfile or fixture that merely sits under `.github`, a nested file GitHub
534        // never reads as a workflow, and a non-YAML extension.
535        assert!(!is_ci_file(Path::new(
536            ".github/selftest/clean/pnpm-lock.yaml"
537        )));
538        assert!(!is_ci_file(Path::new(
539            ".github/workflows/nested/deep/ci.yml"
540        )));
541        assert!(!is_ci_file(Path::new(".github/workflows/notes.md")));
542    }
543
544    #[test]
545    fn an_unreadable_directory_names_itself() {
546        let err = collect_ci_files(
547            Path::new("tc-workflow-lint-no-such-directory"),
548            &mut Vec::new(),
549        )
550        .unwrap_err();
551        assert!(
552            format!("{err:#}").contains("reading directory"),
553            "got: {err:#}"
554        );
555    }
556
557    #[test]
558    fn every_job_in_a_workflow_is_scanned() {
559        let yaml = "jobs:\n  a:\n    steps:\n      - run: for x in 1; do :; done\n  b:\n    steps:\n      - run: cat f | sed s/a/b/\n";
560        let findings = find_violations(yaml, "w.yml");
561        assert_eq!(findings.len(), 2);
562    }
563}
testing_conventions/workflow_lint.rs

testing_conventions/
workflow_lint.rs
