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

testing_conventions/
workflow_lint.rs
