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/// Collect the YAML GitHub actually executes under `path` into `out`: `path` itself when it is a
220/// file, else every qualifying file beneath it, recursively. A named file is always scanned, so a
221/// caller can point the check anywhere.
222fn collect_ci_files(path: &Path, out: &mut Vec<PathBuf>) -> Result<()> {
223    if path.is_file() {
224        out.push(path.to_path_buf());
225        return Ok(());
226    }
227    let entries = std::fs::read_dir(path)
228        .with_context(|| format!("reading directory `{}`", path.display()))?;
229    for entry in entries {
230        let child = crate::walk::dir_entry(entry, path)?.path();
231        if child.is_dir() {
232            collect_ci_files(&child, out)?;
233        } else if is_ci_file(&child) {
234            out.push(child);
235        }
236    }
237    Ok(())
238}
239
240/// `true` when GitHub reads `path` as a workflow or a composite action: a YAML file directly inside
241/// a `workflows` directory, or one named `action.yml` / `action.yaml` anywhere.
242///
243/// Discovery mirrors GitHub's own rules rather than taking every `*.yml` in the tree, so a fixture
244/// or a lockfile that happens to sit under `.github` is never mistaken for CI.
245fn is_ci_file(path: &Path) -> bool {
246    if !matches!(
247        path.extension().and_then(|e| e.to_str()),
248        Some("yml" | "yaml")
249    ) {
250        return false;
251    }
252    let name = path.file_name().and_then(|n| n.to_str());
253    if matches!(name, Some("action.yml" | "action.yaml")) {
254        return true;
255    }
256    path.parent()
257        .and_then(Path::file_name)
258        .and_then(|n| n.to_str())
259        == Some("workflows")
260}
261
262#[cfg(test)]
263mod tests {
264    use super::*;
265
266    #[test]
267    fn straight_line_glue_is_not_logic() {
268        assert_eq!(
269            flag_reasons(
270                "pkg=@x/linux\nmkdir -p \"$pkg/bin\"\ninstall -m 755 bin/x \"$pkg/bin/x\""
271            ),
272            Vec::<String>::new()
273        );
274    }
275
276    #[test]
277    fn a_lone_guard_around_an_exit_is_glue() {
278        let body = "set -euo pipefail\n\
279                    private=$(gh api repos/x --jq .private)\n\
280                    if [ \"$private\" = \"true\" ]; then\n\
281                      echo \"::error::private\"\n\
282                      exit 1\n\
283                    fi\n\
284                    echo ok\n";
285        assert_eq!(flag_reasons(body), Vec::<String>::new());
286    }
287
288    #[test]
289    fn each_loop_and_dispatch_keyword_is_named() {
290        assert_eq!(
291            flag_reasons("for name in $NAMES; do\n  npm publish \"$name\"\ndone"),
292            vec!["for loop"]
293        );
294        assert_eq!(
295            flag_reasons("while read -r l; do\n  echo $l\ndone"),
296            vec!["while loop"]
297        );
298        assert_eq!(
299            flag_reasons("until ok; do\n  sleep 1\ndone"),
300            vec!["until loop"]
301        );
302        assert_eq!(
303            flag_reasons("select x in a b; do\n  echo $x\ndone"),
304            vec!["select loop"]
305        );
306        assert_eq!(
307            flag_reasons("case \"$1\" in\n  a) echo a ;;\nesac"),
308            vec!["case dispatch"]
309        );
310    }
311
312    #[test]
313    fn a_keyword_is_only_a_marker_at_a_command_position() {
314        // `before` / `format` / `casements` share a prefix with a keyword and must not trip it.
315        assert_eq!(
316            flag_reasons("before_hook\nformat_output\ncasements=3"),
317            Vec::<String>::new()
318        );
319        // A keyword as an argument is not a command either.
320        assert_eq!(flag_reasons("echo for while case"), Vec::<String>::new());
321    }
322
323    #[test]
324    fn text_munging_counts_wherever_the_command_runs() {
325        assert_eq!(flag_reasons("awk -F/ '{print $2}' f"), vec!["awk"]);
326        assert_eq!(flag_reasons("echo x | awk -F/ '{print $2}'"), vec!["awk"]);
327        assert_eq!(flag_reasons("cat f | sed 's/a/b/'"), vec!["sed"]);
328        assert_eq!(flag_reasons("v=$(sed -n 1p f)"), vec!["sed"]);
329        assert_eq!(flag_reasons("a; sed -i s/x/y/ f"), vec!["sed"]);
330        assert_eq!(flag_reasons("ok && awk 'END{print NR}' f"), vec!["awk"]);
331    }
332
333    #[test]
334    fn a_munging_command_named_as_an_argument_is_not_running() {
335        assert_eq!(
336            flag_reasons("echo 'use awk for this'"),
337            Vec::<String>::new()
338        );
339        assert_eq!(flag_reasons("apt-get install -y sed"), Vec::<String>::new());
340    }
341
342    #[test]
343    fn a_long_straight_line_body_is_a_script_by_length() {
344        let body = (0..MAX_GLUE_LINES + 3)
345            .map(|i| format!("cmd{i}"))
346            .collect::<Vec<_>>()
347            .join("\n");
348        assert_eq!(flag_reasons(&body), vec!["15 lines (> 12)"]);
349    }
350
351    #[test]
352    fn the_length_count_ignores_bookkeeping() {
353        // Exactly at the ceiling passes, and the prologue, comments, and blanks do not count.
354        let body = (0..MAX_GLUE_LINES)
355            .map(|i| format!("cmd{i}"))
356            .collect::<Vec<_>>()
357            .join("\n");
358        assert_eq!(flag_reasons(&body), Vec::<String>::new());
359        assert_eq!(
360            flag_reasons("set -euo pipefail\n# a note\n\necho hi"),
361            Vec::<String>::new()
362        );
363    }
364
365    #[test]
366    fn every_reason_a_body_earns_is_reported() {
367        let body = "for f in *; do\n  case $f in\n    a) sed -i s/x/y/ $f ;;\n  esac\ndone";
368        assert_eq!(flag_reasons(body), vec!["for loop", "case dispatch", "sed"]);
369    }
370
371    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";
372
373    #[test]
374    fn a_run_step_reports_its_file_name_and_line() {
375        let findings = find_violations(LOOP_WORKFLOW, "w.yml");
376        assert_eq!(findings.len(), 1);
377        assert_eq!(findings[0].step, "Publish stubs");
378        assert_eq!(findings[0].kind, "run");
379        assert_eq!(findings[0].reasons, vec!["for loop"]);
380        assert_eq!(findings[0].file, PathBuf::from("w.yml"));
381        assert_eq!(findings[0].line, 6, "the body starts on line 6");
382    }
383
384    #[test]
385    fn a_github_script_body_is_judged_too() {
386        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";
387        let findings = find_violations(yaml, "w.yml");
388        assert_eq!(findings.len(), 1);
389        assert_eq!(findings[0].kind, "github-script");
390        assert_eq!(findings[0].step, "actions/github-script@v7");
391        assert_eq!(findings[0].reasons, vec!["for loop"]);
392    }
393
394    #[test]
395    fn another_actions_script_input_is_not_a_github_script_body() {
396        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";
397        assert_eq!(find_violations(yaml, "w.yml"), Vec::new());
398    }
399
400    #[test]
401    fn a_composite_action_is_scanned_through_runs_steps() {
402        let yaml = "runs:\n  using: composite\n  steps:\n    - name: Munge\n      shell: bash\n      run: cat f | sed 's/a/b/'\n";
403        let findings = find_violations(yaml, "action.yml");
404        assert_eq!(findings.len(), 1);
405        assert_eq!(findings[0].step, "Munge");
406        assert_eq!(findings[0].reasons, vec!["sed"]);
407    }
408
409    #[test]
410    fn a_clean_workflow_yields_nothing() {
411        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";
412        assert_eq!(find_violations(yaml, "w.yml"), Vec::new());
413    }
414
415    #[test]
416    fn a_step_without_a_name_falls_back_to_its_uses_then_a_placeholder() {
417        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";
418        assert_eq!(
419            find_violations(named_by_uses, "w.yml")[0].step,
420            "actions/setup-node@v4"
421        );
422
423        let unnamed = "jobs:\n  j:\n    steps:\n      - run: for x in 1; do echo $x; done\n";
424        assert_eq!(find_violations(unnamed, "w.yml")[0].step, "<unnamed step>");
425    }
426
427    #[test]
428    fn unparseable_yaml_is_actionlints_to_report_not_ours() {
429        assert_eq!(find_violations("jobs: [unclosed\n", "w.yml"), Vec::new());
430    }
431
432    #[test]
433    fn a_document_with_no_steps_anywhere_yields_nothing() {
434        assert_eq!(find_violations("name: CI\non: push\n", "w.yml"), Vec::new());
435        assert_eq!(find_violations("- a\n- b\n", "w.yml"), Vec::new());
436        assert_eq!(find_violations("", "w.yml"), Vec::new());
437        // A job, or a composite `runs:`, that declares no steps at all.
438        assert_eq!(
439            find_violations("jobs:\n  a:\n    runs-on: ubuntu-latest\n", "w.yml"),
440            Vec::new()
441        );
442        assert_eq!(
443            find_violations("runs:\n  using: node20\n  main: index.js\n", "action.yml"),
444            Vec::new()
445        );
446    }
447
448    #[test]
449    fn a_job_that_is_not_a_mapping_is_skipped() {
450        // `jobs:` holding a scalar is nonsense GitHub would reject; it must not panic here.
451        assert_eq!(find_violations("jobs:\n  a: 3\n", "w.yml"), Vec::new());
452    }
453
454    #[test]
455    fn a_github_script_step_carrying_no_script_has_no_body() {
456        let yaml = "jobs:\n  j:\n    steps:\n      - uses: actions/github-script@v7\n";
457        assert_eq!(find_violations(yaml, "w.yml"), Vec::new());
458    }
459
460    #[test]
461    fn discovery_follows_githubs_own_rules() {
462        assert!(is_ci_file(Path::new(".github/workflows/ci.yml")));
463        assert!(is_ci_file(Path::new(".github/workflows/ci.yaml")));
464        assert!(is_ci_file(Path::new(".github/actions/detect/action.yml")));
465        assert!(is_ci_file(Path::new("tools/thing/action.yaml")));
466
467        // Not CI: a lockfile or fixture that merely sits under `.github`, a nested file GitHub
468        // never reads as a workflow, and a non-YAML extension.
469        assert!(!is_ci_file(Path::new(
470            ".github/selftest/clean/pnpm-lock.yaml"
471        )));
472        assert!(!is_ci_file(Path::new(
473            ".github/workflows/nested/deep/ci.yml"
474        )));
475        assert!(!is_ci_file(Path::new(".github/workflows/notes.md")));
476    }
477
478    #[test]
479    fn an_unreadable_directory_names_itself() {
480        let err = collect_ci_files(
481            Path::new("tc-workflow-lint-no-such-directory"),
482            &mut Vec::new(),
483        )
484        .unwrap_err();
485        assert!(
486            format!("{err:#}").contains("reading directory"),
487            "got: {err:#}"
488        );
489    }
490
491    #[test]
492    fn every_job_in_a_workflow_is_scanned() {
493        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";
494        let findings = find_violations(yaml, "w.yml");
495        assert_eq!(findings.len(), 2);
496    }
497}
testing_conventions/workflow_lint.rs

testing_conventions/
workflow_lint.rs
