|
| 1 | +#!/usr/bin/env bash |
| 2 | +# |
| 3 | +# Asserts the workflows are valid to GitHub, not merely valid YAML. |
| 4 | +# |
| 5 | +# This file exists because of an outage. A shell comment inside a `run:` block explained that |
| 6 | +# PR title and body deliberately avoid an Actions expression -- and spelled the delimiter out |
| 7 | +# to say so. Actions parses those delimiters everywhere in a workflow file, including inside |
| 8 | +# a run block's shell comments, and an empty pair is a syntax error. The workflow became |
| 9 | +# unparseable, so every run started with zero jobs, the required check never reported, and |
| 10 | +# every open pull request across the org sat behind "Please close and reopen the PR to |
| 11 | +# trigger this workflow" until the comment was reworded. |
| 12 | +# |
| 13 | +# Nothing in the previous suite could see it. `yaml.safe_load` accepts the file, the shell |
| 14 | +# runs the comment happily, and tests/context-step-test.sh explicitly *exempted* comments |
| 15 | +# from its delimiter check. The lesson is narrow and worth encoding: YAML-valid is not |
| 16 | +# Actions-valid, and the gap is expression syntax. |
| 17 | +# |
| 18 | +# actionlint is the real check and runs when available. The scan below is the part that |
| 19 | +# always runs, because CI must not depend on a tool being installed to catch the specific |
| 20 | +# defect that caused an outage. |
| 21 | + |
| 22 | +set -euo pipefail |
| 23 | + |
| 24 | +cd "$(dirname "$0")/.." |
| 25 | + |
| 26 | +failures=0 |
| 27 | +WORKFLOW_FILE=.github/workflows/claude-pr-review.yml |
| 28 | + |
| 29 | +# The delimiter, assembled rather than written, so this file does not trip its own scan. |
| 30 | +OPEN="\${$(printf '%s' '{')" |
| 31 | +CLOSE="$(printf '%s' '}')}" |
| 32 | + |
| 33 | +for wf in .github/workflows/*.yml; do |
| 34 | + # The distinction that caused the outage, and the one the scan has to make: a delimiter in |
| 35 | + # a *YAML* comment is stripped by the YAML parser and never reaches Actions, while the same |
| 36 | + # characters inside a block scalar are part of the string value and are parsed. So block |
| 37 | + # scalars are scanned line for line, comments included, and outside them YAML comment lines |
| 38 | + # are skipped. Scanning raw text without that distinction reports the harmless env: comment |
| 39 | + # that has sat in this workflow since before any of this and would train the reader to |
| 40 | + # ignore the check. |
| 41 | + bad=$(awk -v open="$OPEN" -v shut="$CLOSE" ' |
| 42 | + function scan(line, where, i, rest, j, expr) { |
| 43 | + while ((i = index(line, open)) > 0) { |
| 44 | + rest = substr(line, i + length(open)) |
| 45 | + j = index(rest, shut) |
| 46 | + if (j == 0) { print FILENAME ":" FNR ": unterminated expression (" where ")"; return } |
| 47 | + expr = substr(rest, 1, j - 1) |
| 48 | + gsub(/^[ \t]+|[ \t]+$/, "", expr) |
| 49 | + if (expr == "") |
| 50 | + print FILENAME ":" FNR ": empty expression in " where \ |
| 51 | + " -- Actions rejects the whole workflow" |
| 52 | + else if (expr !~ /^[A-Za-z_0-9.,()!<>=&|*'\''"\[\] \t-]+$/) |
| 53 | + print FILENAME ":" FNR ": odd expression in " where ": " expr |
| 54 | + line = substr(rest, j + length(shut)) |
| 55 | + } |
| 56 | + } |
| 57 | + { |
| 58 | + indent = match($0, /[^ ]/) - 1 |
| 59 | + if (indent < 0) indent = length($0) |
| 60 | + if (in_block) { |
| 61 | + if ($0 ~ /^[ \t]*$/) next |
| 62 | + if (indent < block_indent) in_block = 0 |
| 63 | + else { scan($0, "a block scalar (a shell comment counts)"); next } |
| 64 | + } |
| 65 | + if ($0 ~ /:[ \t]*[|>][-+0-9]*[ \t]*$/) { |
| 66 | + in_block = 1 |
| 67 | + block_indent = indent + 1 |
| 68 | + next |
| 69 | + } |
| 70 | + # Outside a block scalar a whole-line YAML comment is invisible to Actions. |
| 71 | + if ($0 ~ /^[ \t]*#/) next |
| 72 | + scan($0, "a value") |
| 73 | + } |
| 74 | + ' "$wf") || { |
| 75 | + echo "FAIL the expression scan itself failed on $wf; the scan proves nothing" |
| 76 | + failures=$((failures + 1)) |
| 77 | + continue |
| 78 | + } |
| 79 | + if [ -n "$bad" ]; then |
| 80 | + echo "FAIL $wf has invalid Actions expressions:" |
| 81 | + printf '%s\n' "$bad" | sed 's/^/ /' |
| 82 | + failures=$((failures + 1)) |
| 83 | + else |
| 84 | + echo "ok $wf expressions are well formed" |
| 85 | + fi |
| 86 | +done |
| 87 | + |
| 88 | +# Every read the context step makes needs a permission declared on the job, because the job |
| 89 | +# declares `permissions:` explicitly and anything unlisted is `none`. That failure is silent |
| 90 | +# by design -- each block degrades to its "could not read" sentence -- so a missing line here |
| 91 | +# does not turn a run red, it just quietly empties the block. Two were missing on the first |
| 92 | +# production run (checks and statuses), and no local test could see it: a personal access |
| 93 | +# token has every scope, so the step works on a laptop and fails on the runner. |
| 94 | +# |
| 95 | +# The table is endpoint-shape to permission. It is deliberately coarse; the point is that |
| 96 | +# adding a new API call to the step forces a decision about its permission. |
| 97 | +step_script=$(awk ' |
| 98 | + /^ - name: Gather review context$/ { in_step = 1 } |
| 99 | + in_step && /^ run: \|$/ { in_run = 1; next } |
| 100 | + in_run && /^ [a-z]/ { exit } |
| 101 | + in_run { print } |
| 102 | +' "$WORKFLOW_FILE") |
| 103 | +declared=$(awk '/^ permissions:$/ { p = 1; next } p && /^ [a-z-]+:/ { print $1 } p && /^ [a-z]/ { exit }' \ |
| 104 | + "$WORKFLOW_FILE" | tr -d ':') |
| 105 | + |
| 106 | +# check <pattern> <permission> <what it is for> |
| 107 | +check_permission() { |
| 108 | + printf '%s\n' "$step_script" | grep -qF -- "$1" || return 0 |
| 109 | + if printf '%s\n' "$declared" | grep -qx "$2"; then |
| 110 | + echo "ok $2 is declared for $3" |
| 111 | + else |
| 112 | + echo "FAIL the step calls $3 but the job never grants $2: read" |
| 113 | + failures=$((failures + 1)) |
| 114 | + fi |
| 115 | +} |
| 116 | + |
| 117 | +check_permission "/actions/jobs/" actions "the failing-job log excerpts" |
| 118 | +check_permission "/issues/" issues "the PR conversation comments" |
| 119 | +check_permission "statusCheckRollup" checks "the CheckRun half of the CI rollup" |
| 120 | +check_permission "statusCheckRollup" statuses "the StatusContext half of the CI rollup" |
| 121 | +check_permission "/compare/" contents "the since-last-review comparison" |
| 122 | +check_permission "/pulls/" pull-requests "the PR reads" |
| 123 | + |
| 124 | +# The scan above is a backstop for one class. actionlint checks the schema, the expression |
| 125 | +# grammar, and the shell; run it when it is on PATH. shellcheck findings are excluded because |
| 126 | +# the run blocks here intentionally use unquoted word splitting for job ids. |
| 127 | +if command -v actionlint >/dev/null 2>&1; then |
| 128 | + if out=$(actionlint .github/workflows/*.yml 2>&1); then |
| 129 | + echo "ok actionlint reports no findings" |
| 130 | + else |
| 131 | + remaining=$(printf '%s\n' "$out" | grep -v 'shellcheck reported' || true) |
| 132 | + if printf '%s\n' "$remaining" | grep -qE '\[(expression|syntax-check|events|workflow-call)\]'; then |
| 133 | + echo "FAIL actionlint reports errors that would stop the workflow from starting:" |
| 134 | + printf '%s\n' "$remaining" | grep -E '\[(expression|syntax-check|events|workflow-call)\]' \ |
| 135 | + | sed 's/^/ /' |
| 136 | + failures=$((failures + 1)) |
| 137 | + else |
| 138 | + echo "ok actionlint reports no startup-fatal findings" |
| 139 | + fi |
| 140 | + fi |
| 141 | +else |
| 142 | + echo "skip actionlint not installed; only the expression scan ran" |
| 143 | +fi |
| 144 | + |
| 145 | +if [ "$failures" -ne 0 ]; then |
| 146 | + echo "$failures test(s) failed" |
| 147 | + exit 1 |
| 148 | +fi |
| 149 | +echo "all tests passed" |
0 commit comments