diff --git a/.github/workflows/issues-add-labels.yaml b/.github/workflows/issues-add-labels.yaml index 11703b0..2565283 100644 --- a/.github/workflows/issues-add-labels.yaml +++ b/.github/workflows/issues-add-labels.yaml @@ -1,7 +1,12 @@ name: Label issues -permissions: - contents: read +# Empty on purpose. A caller can only hand down what its own job has, and the +# stub in the README calls this with `issues: write` alone — a job-level block +# replaces the workflow-level one rather than merging, so the caller's job has +# no `contents`. Asking for it here fails the caller at startup, before a +# single job runs. +permissions: {} + on: issues: types: @@ -11,6 +16,8 @@ on: jobs: label_issues: runs-on: ubuntu-24.04 + # `gh issue edit` needs issues:write and nothing else — no checkout, no + # other API surface. permissions: issues: write steps: diff --git a/.github/workflows/test-actions.yml b/.github/workflows/test-actions.yml index 5e49622..5e1634e 100644 --- a/.github/workflows/test-actions.yml +++ b/.github/workflows/test-actions.yml @@ -2,15 +2,17 @@ name: Test Actions # Consuming repositories pin this repo by SHA, so anything shipped from here # reaches their CI unchanged. The triggers therefore cover what is shipped — -# the composite actions and their pins, common.mk, the scripts and the flake — -# not just the shell scripts. A pin bump in an action.yml used to change what -# every consumer runs without a single job executing here. +# the composite actions and their pins, common.mk, the scripts, the flake, the +# reusable workflows, and the README caller stubs consumers copy — not just the +# shell scripts. A pin bump in an action.yml used to change what every consumer +# runs without a single job executing here, and a permissions change in a +# reusable workflow can break every caller at startup. on: pull_request: paths: - '.github/actions/**' - - '.github/workflows/test-actions.yml' - - '.github/workflows/update-action-pins.yml' + - '.github/workflows/**' + - 'README.md' - 'common.mk' - 'flake.lock' - 'flake.nix' @@ -19,8 +21,8 @@ on: branches: [main] paths: - '.github/actions/**' - - '.github/workflows/test-actions.yml' - - '.github/workflows/update-action-pins.yml' + - '.github/workflows/**' + - 'README.md' - 'common.mk' - 'flake.lock' - 'flake.nix' @@ -53,6 +55,25 @@ jobs: - name: Test check-dev-kit-pins.sh run: scripts/test-check-dev-kit-pins.sh + + # yq is not on the runner image but is in this repository's dev shell, so + # both steps run through it. + workflow-permissions: + needs: shellcheck + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + - name: Set up nix with the shared Cachix cache + uses: ./.github/actions/setup-nix + with: + cachix-auth-token: ${{ secrets.CACHIX_AUTH_TOKEN }} + cachix-signing-key: ${{ secrets.CACHIX_SIGNING_KEY }} + - name: Test check-reusable-permissions.sh + run: nix develop --command scripts/test-check-reusable-permissions.sh + # Dogfood: the same rule against this repository's own reusable workflows, + # its in-repo callers, and the stubs in the README that consumers copy. + - name: This repository's own workflow permissions agree with its callers + run: nix develop --command scripts/check-reusable-permissions.sh test-setup-nix: needs: shellcheck runs-on: ubuntu-24.04 diff --git a/scripts/check-reusable-permissions.sh b/scripts/check-reusable-permissions.sh new file mode 100755 index 0000000..44cb218 --- /dev/null +++ b/scripts/check-reusable-permissions.sh @@ -0,0 +1,200 @@ +#!/usr/bin/env bash +# Check that no reusable workflow asks for more GITHUB_TOKEN permissions than +# its callers hand it. +# +# Two GitHub rules make this a recurring, silent failure: +# +# 1. A called workflow can never elevate: it gets at most what the calling +# *job* was granted. +# 2. A job-level `permissions:` block REPLACES the workflow-level one. It does +# not merge. A job that sets only `issues: write` therefore has +# `contents: none`, whatever the workflow level says. +# +# Breaking either fails the caller at *startup*, before any job runs, which +# creates no check run — so nothing goes red. `actionlint` exits 0 on it too. +# That combination is why this has needed three separate fixes. +# +# Callers come from two places and both are checked: +# - in-repo: a job with `uses: ./.github/workflows/` +# - consumers: the stubs in README.md, which is what consumers copy +# +# Checking only the README would miss an in-repo caller; checking only in-repo +# callers would miss every consumer. Both have bitten. +# +# yq is used only to turn YAML into JSON — its expression language has no +# `if`/`def` — so the rules above live in one jq program. +# +# Usage: check-reusable-permissions.sh [repo-dir] +set -euo pipefail + +# yq is not on the GitHub runner image; jq is. Both are in this repository's +# dev shell, which is how CI runs this. +for bin in yq jq; do + if ! command -v "$bin" > /dev/null 2>&1; then + echo "error: $bin is required but not on PATH." >&2 + echo " In this repository: nix develop --command $0" >&2 + exit 1 + fi +done + +cd "${1:-.}" + +WORKFLOW_DIR="${WORKFLOW_DIR:-.github/workflows}" +README="${README_FILE:-README.md}" +# Which qualified `uses:` paths count as this repository. A bare substring match +# would also select another repository's workflow that happens to share a +# filename with one of ours, and compare the wrong pair. +REPO_SLUG="${REPO_SLUG:-opendefensecloud/dev-kit}" + +tmp="$(mktemp -d)" +trap 'rm -rf "$tmp"' EXIT + +: > "$tmp/docs.ndjson" + +# A file this script cannot parse has to stop it. Skipping one quietly turns a +# real failure into a pass: the broken caller simply disappears, and its callee +# is then reported as "nothing calls it". +for f in "$WORKFLOW_DIR"/*.y*ml; do + [ -f "$f" ] || continue + if ! yq -o=json -I=0 '.' "$f" > "$tmp/doc.json" 2> "$tmp/yqerr"; then + echo "error: cannot parse $f as YAML:" >&2 + sed 's/^/ /' "$tmp/yqerr" >&2 + exit 1 + fi + jq -c --arg name "$(basename "$f")" --arg label "$f" \ + '{name: $name, label: $label, local: true, doc: .}' \ + < "$tmp/doc.json" >> "$tmp/docs.ndjson" +done + +if [ -f "$README" ]; then + # Split the README's fenced yaml blocks out so each can be parsed on its own. + awk -v d="$tmp" ' + /^```yaml$/ { n++; inb = 1; next } + /^```$/ { inb = 0; next } + inb && n { print > (d "/readme-block-" n ".yaml") } + ' "$README" + for b in "$tmp"/readme-block-*.yaml; do + [ -f "$b" ] || continue + if ! yq -o=json -I=0 '.' "$b" > "$tmp/doc.json" 2> "$tmp/yqerr"; then + # A README may hold illustrative fragments that do not stand alone. One + # that calls our workflows may not: that is the snippet consumers copy, + # so it has to be checkable. + if grep -qF "$REPO_SLUG/.github/workflows/" "$b"; then + echo "error: a yaml block in $README calls $REPO_SLUG but does not parse:" >&2 + sed 's/^/ /' "$tmp/yqerr" >&2 + sed 's/^/ | /' "$b" >&2 + exit 1 + fi + continue + fi + jq -c --arg label "$README (stub)" \ + 'select(type == "object" and has("jobs")) | {name: null, label: $label, local: false, doc: .}' \ + < "$tmp/doc.json" >> "$tmp/docs.ndjson" + done +fi + +jq -rs --arg slug "$REPO_SLUG" ' + def lvl: {"none": 1, "read": 2, "write": 3}[.] // 0; + + # `permissions: read-all` and `permissions: write-all` are valid at both the + # workflow and the job level. Normalising them onto a wildcard key lets one + # lookup rule cover every case: a grant of read-all answers any single scope, + # while a *request* for read-all is satisfied only by a caller that also + # granted every scope. + def norm: + if . == null then null + elif type == "string" then + (if . == "read-all" then {"*": "read"} + elif . == "write-all" then {"*": "write"} + else {} end) + else . end; + + # What a grant gives for one scope, falling back to a wildcard grant. + def at($k): (.[$k] // .["*"] // "none"); + + # Strongest request per key across a list of permission maps. A caller grant + # caps the called workflow at every level, so the maximum is what matters. + def strongest(maps): reduce (maps | .[] | to_entries[]) as $e ({}; + if ($e.value | lvl) > ((.[$e.key] // "none") | lvl) then .[$e.key] = $e.value else . end); + + # A calling job gets its own block when it has one, the workflow-level block + # otherwise. Checked against null, not with `//`: an explicit + # `permissions: {}` grants nothing and must not fall back to the workflow. + def grant($top): (if .permissions != null then .permissions else $top end) | norm; + + . as $docs + + # callee name -> the strongest permissions it asks for anywhere + | [ $docs[] + | select(.local and (.doc.jobs != null) and ((.doc["on"] // {}) | has("workflow_call"))) + | { name: .name, + requires: strongest([ ((.doc.permissions | norm) // {}) ] + + [ .doc.jobs[] | ((.permissions | norm) // {}) ]) } + ] as $callees + + # every (caller job -> callee) edge, with the effective grant of that job + | [ $docs[] + | .label as $label + | (.doc.permissions) as $top + | (.doc.jobs // {}) | to_entries[] + | . as $j + | (.value.uses // "") as $uses + | select(($uses | startswith("./.github/workflows/")) + or ($uses | startswith($slug + "/.github/workflows/"))) + | { callee: ($uses | sub("@.*"; "") | sub(".*/"; "")), + label: "\($label) (\($j.key))", + grant: ($j.value | grant($top)) } + ] as $edges + + | $callees[] + | . as $callee + | [ $edges[] | select(.callee == $callee.name) ] as $mine + | if ($mine | length) == 0 then + "WARN|\($callee.name)|nothing calls it in this repo or the README|" + else + $mine[] + | if .grant == null then + "WARN|\($callee.name)|\(.label)|declares no permissions, so the repository default decides" + else + .grant as $g + | [ $callee.requires | to_entries[] + | .key as $k + | .value as $v + | select(($v | lvl) > (($g | at($k)) | lvl)) + | (if $k == "*" then "all scopes" else $k end) as $label + | "\($label): wants \($v), caller grants \($g | at($k))" + ] as $bad + | if ($bad | length) == 0 + then "ok|\($callee.name)|\(.label)|" + else "FAIL|\($callee.name)|\(.label)|\($bad | join("; "))" + end + end + end +' "$tmp/docs.ndjson" > "$tmp/report" + +if [ ! -s "$tmp/report" ]; then + echo "error: found no reusable workflows under $WORKFLOW_DIR." >&2 + exit 1 +fi + +rc=0 +while IFS='|' read -r status callee label detail; do + if [ "$status" = "FAIL" ]; then rc=1; fi + if [ -n "$detail" ]; then + printf '%-5s %-30s %s -- %s\n' "$status" "$callee" "$label" "$detail" + else + printf '%-5s %-30s %s\n' "$status" "$callee" "$label" + fi +done < "$tmp/report" + +if [ "$rc" -ne 0 ]; then + cat >&2 << 'EOF' + +A called workflow cannot ask for more than its calling job was granted, and a +job-level `permissions:` block replaces the workflow-level one instead of +merging. Either drop the request from the called workflow, or widen the calling +job -- listing every key it needs, not only the one that is missing. +EOF +fi + +exit "$rc" diff --git a/scripts/test-check-reusable-permissions.sh b/scripts/test-check-reusable-permissions.sh new file mode 100755 index 0000000..ab8bef4 --- /dev/null +++ b/scripts/test-check-reusable-permissions.sh @@ -0,0 +1,244 @@ +#!/usr/bin/env bash +# Fixtures for check-reusable-permissions.sh. +# +# dev-kit dogfoods this check: CI runs it against this repository own workflows. +# But the interesting cases are the broken ones, which must not be committed. +# So each case builds a miniature repo in a temp directory: one called workflow, +# one caller, and the README stub a consumer would copy. +set -euo pipefail + +ROOT="$(cd "$(dirname "$0")" && pwd)" +TESTDIR="$(mktemp -d)" +trap 'rm -rf "$TESTDIR"' EXIT + +cd "$TESTDIR" + +run_test() { + local name="$1" expected_rc="$2" expected_output="$3" + + echo "=== $name ===" + local output rc + set +e + output="$(bash "$ROOT/check-reusable-permissions.sh" "$TESTDIR" 2>&1)" + rc=$? + set -e + if [ -n "$output" ]; then + echo "$output" + fi + if [ "$rc" -ne "$expected_rc" ]; then + echo "FAIL: expected exit code $expected_rc, got $rc" + exit 1 + fi + if echo "$output" | grep -q "$expected_output"; then + echo "PASS" + else + echo "FAIL: expected output to contain '$expected_output'" + exit 1 + fi + echo +} + +reset() { + rm -rf .github README.md + mkdir -p .github/workflows +} + +# emit_perms +# "-" omits the block. A value with a colon is a mapping written on the +# following lines; one without is a scalar on the same line, which is how +# `permissions: read-all` and `permissions: {}` are spelled. +emit_perms() { + local ind="$1" val="$2" + [ "$val" = "-" ] && return 0 + case "$val" in + *:*) + printf '%spermissions:\n' "$ind" + printf '%s\n' "$val" | sed "s/^/${ind} /" + ;; + *) printf '%spermissions: %s\n' "$ind" "$val" ;; + esac +} + +# callee +callee() { + { + echo "name: Callee" + emit_perms "" "$1" + echo "on:" + echo " workflow_call:" + echo "jobs:" + echo " work:" + echo " runs-on: ubuntu-24.04" + emit_perms " " "$2" + echo " steps:" + echo " - run: 'true'" + } > .github/workflows/callee.yml +} + +# local_caller +local_caller() { + { + echo "name: Local caller" + emit_perms "" "$1" + echo "on:" + echo " pull_request:" + echo "jobs:" + echo " call:" + emit_perms " " "$2" + echo " uses: ./.github/workflows/callee.yml" + } > .github/workflows/local-caller.yml +} + +# readme_stub +readme_stub() { + { + echo "# Fixture" + echo + echo '```yaml' + echo "name: Consumer" + emit_perms "" "$1" + echo "on:" + echo " pull_request:" + echo "jobs:" + echo " consume:" + emit_perms " " "$2" + echo " uses: opendefensecloud/dev-kit/.github/workflows/callee.yml@0000000000000000000000000000000000000000 # v0.0.0" + echo '```' + } > README.md +} + +# --------------------------------------------------------------------------- +# The two rules +# --------------------------------------------------------------------------- + +reset +callee "contents: read" "-" +local_caller "contents: read" "-" +run_test "caller job inherits a workflow-level grant that is enough" 0 "ok callee.yml" + +reset +callee "contents: read" "-" +local_caller "contents: read" "contents: write" +run_test "write satisfies a request for read" 0 "ok callee.yml" + +# The #53 class: the job block replaces the workflow block, so contents is gone. +reset +callee "contents: read" "issues: write" +local_caller "contents: read" "issues: write" +run_test "job-level block replaces rather than merges" 1 "contents: wants read, caller grants none" + +reset +callee "contents: read" "-" +local_caller "contents: read" "{}" +run_test "explicit empty grant does not fall back to the workflow level" 1 "contents: wants read, caller grants none" + +reset +callee "{}" "issues: write" +local_caller "contents: read" "issues: write" +run_test "callee asking for nothing at the workflow level passes" 0 "ok callee.yml" + +# A called workflow cannot elevate at its job level either. +reset +callee "{}" "pull-requests: write" +local_caller "contents: read" "contents: read" +run_test "callee job-level request is capped by the caller too" 1 "pull-requests: wants write, caller grants none" + +reset +callee "actions: read +contents: read" "-" +local_caller "contents: read" "contents: read +security-events: write" +run_test "only the missing key is reported" 1 "actions: wants read, caller grants none" + +# --------------------------------------------------------------------------- +# read-all / write-all, which are valid at both levels +# --------------------------------------------------------------------------- + +reset +callee "read-all" "-" +local_caller "contents: read" "contents: read" +run_test "a callee wanting read-all is not satisfied by one scope" 1 "all scopes: wants read, caller grants none" + +reset +callee "read-all" "-" +local_caller "contents: read" "read-all" +run_test "a callee wanting read-all is satisfied by read-all" 0 "ok callee.yml" + +reset +callee "contents: read" "-" +local_caller "contents: read" "write-all" +run_test "write-all satisfies a request for one scope" 0 "ok callee.yml" + +# --------------------------------------------------------------------------- +# Who counts as a caller +# --------------------------------------------------------------------------- + +reset +callee "contents: read" "-" +local_caller "contents: read" "contents: read" +readme_stub "contents: read" "issues: write" +run_test "a broken README stub fails even when the in-repo caller is fine" 1 "README.md (stub) (consume)" + +reset +callee "contents: read" "-" +local_caller "contents: read" "contents: read" +readme_stub "contents: read" "contents: read" +run_test "both callers correct" 0 "ok callee.yml" + +reset +callee "contents: read" "-" +local_caller "-" "-" +run_test "caller declaring nothing anywhere warns instead of failing" 0 "WARN callee.yml" + +reset +callee "contents: read" "-" +run_test "a reusable workflow nobody calls warns" 0 "nothing calls it in this repo or the README" + +# Another repository's workflow can share a filename with ours; its grant must +# not be compared against our callee. +reset +callee "contents: read" "-" +local_caller "contents: read" "contents: read" +cat >> .github/workflows/local-caller.yml << 'EOF' + theirs: + permissions: {} + uses: someorg/unrelated/.github/workflows/callee.yml@0000000000000000000000000000000000000000 +EOF +run_test "a foreign workflow with the same filename is not a caller" 0 "ok callee.yml" + +# --------------------------------------------------------------------------- +# Unparseable input must stop the check, not shrink it +# --------------------------------------------------------------------------- + +# The broken file is also the broken caller, so swallowing the parse error +# would turn a FAIL into a pass with the callee reported as uncalled. +reset +callee "contents: read" "-" +local_caller "contents: read" "issues: write" +echo ' oops: [this is: not valid' >> .github/workflows/local-caller.yml +run_test "a workflow that does not parse fails the check" 1 "cannot parse" + +reset +callee "contents: read" "-" +local_caller "contents: read" "contents: read" +readme_stub "contents: read" "contents: read" +# Break the stub that calls us. +sed -i 's|^on:$|on: [this is: not valid|' README.md +run_test "a README stub that calls us and does not parse fails the check" 1 "does not parse" + +# An illustrative fragment that does not call us may legitimately not stand +# alone, and must not fail the check. +reset +callee "contents: read" "-" +local_caller "contents: read" "contents: read" +{ + echo "# Fixture" + echo + echo '```yaml' + echo "# a fragment, not a whole workflow" + echo "with: {scan-args: |-" + echo '```' +} > README.md +run_test "an unparseable fragment that does not call us is ignored" 0 "ok callee.yml" + +echo "All checks passed."