Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 31 additions & 11 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,10 @@ jobs:
# Used by `gh api .../pulls/N/files` in changed-only mode. If the
# caller's GITHUB_TOKEN doesn't grant pull-requests, this job-level
# request is silently capped — the gh api call 403s and the
# "Resolve prettier targets" step gracefully falls back to full-glob
# mode (emits ::warning:: and sets mode=glob). The workflow still
# "Resolve prettier targets" step retries, warns, and skips the
# prettier check for that run (sets mode=none — a scoping failure
# is never widened to a full-tree check, which would go red on
# pre-existing drift in files the PR never touched). The workflow still
# starts because this scope is requested at job level, not workflow
# level — workflow-level perms are a hard ceiling that cause
# startup_failure if exceeded by the caller's grant.
Expand Down Expand Up @@ -200,13 +202,14 @@ jobs:

# Decide what files prettier should check. Three modes:
# - mode=glob → existing behavior, run prettier --check "$GLOB"
# (push events, or PRs with prettier_changed_only=false,
# or PRs where listing changed files failed)
# (push events, or PRs with prettier_changed_only=false)
# - mode=files → run prettier --check on an explicit file list
# (PRs, default — only files added/modified/renamed
# in the diff that also match the glob)
# - mode=none → no targets; emit a no-op log line and pass
# (a PR that changed zero markdown files)
# (a PR that changed zero markdown files, or the
# changed-file listing failed after retries —
# fail-safe, see the listing block below)
#
# Glob ∩ changed-files intersection is computed by:
# 1. Asking GitHub for the PR's file list (excludes deletions).
Expand Down Expand Up @@ -245,14 +248,31 @@ jobs:
exit 0
fi

# List the PR's changed files, with retries — the API can hiccup
# (an HTML 5xx body makes gh's --jq die with "invalid character
# '<' looking for beginning of value"). On persistent failure
# SKIP the prettier check for this run instead of falling open
# to the full-tree glob: a full-tree check goes red on
# pre-existing drift in files the PR never touched. Block below
# is byte-identical in prettier-autofix.yml — the selftest
# drift-checks it; keep both in sync.
PR_FILES_LOG="$(mktemp)"
if ! gh api --paginate \
"repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/files" \
--jq '.[] | select(.status != "removed") | .filename' \
> "$PR_FILES_LOG" 2>/tmp/gh_err; then
echo "::warning::Could not list PR files (often means the caller's GITHUB_TOKEN lacks 'pull-requests: read'). Falling back to full-glob mode."
LISTED=0
for attempt in 1 2 3; do
if gh api --paginate \
"repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/files" \
--jq '.[] | select(.status != "removed") | .filename' \
> "$PR_FILES_LOG" 2>/tmp/gh_err; then
LISTED=1
break
fi
echo "Changed-file listing attempt ${attempt}/3 failed:"
sed 's/^/ /' /tmp/gh_err || true
echo "mode=glob" >> "$GITHUB_OUTPUT"
if [ "$attempt" -lt 3 ]; then sleep "$(( attempt * 2 ))"; fi
done
if [ "$LISTED" -ne 1 ]; then
echo "::warning::Could not list PR files after 3 attempts (transient API error, or the caller's GITHUB_TOKEN lacks 'pull-requests: read'). Skipping prettier for this run — refusing to widen scope to the full tree."
echo "mode=none" >> "$GITHUB_OUTPUT"
exit 0
fi
mapfile -t CHANGED < "$PR_FILES_LOG"
Expand Down
51 changes: 45 additions & 6 deletions .github/workflows/prettier-autofix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ name: Prettier Autofix
# token" — a permanent red X on every dependabot PR. Mirrors the
# bot-skip in claude-review.yml. Dependency bumps don't need autofix.
# - PRs that touched zero markdown files: skipped (no-op).
# - Changed-file listing failures (transient API errors): skipped after
# 3 attempts — a scoping failure is never widened to a full-tree run
# (wxa-secrets#27: the old full-tree fallback reformatted pre-existing
# drift in files the PR never touched and the push died on workflow
# scope).
# - Writes under .github/workflows/ are always reverted before commit:
# automerge_pat deliberately lacks workflow scope (fleet policy), so
# GitHub rejects any push containing them.
# - PRs already cleanly formatted: skipped (no commit pushed).

on:
Expand Down Expand Up @@ -149,14 +157,33 @@ jobs:
exit 0
fi

# List the PR's changed files, with retries — the API can hiccup
# (an HTML 5xx body makes gh's --jq die with "invalid character
# '<' looking for beginning of value"; observed on
# whois-api-llc/wxa-secrets#27). On persistent failure SKIP this
# run instead of falling open to the full-tree glob: a full-tree
# pass drags in pre-existing drift from files the PR never
# touched, and those writes can be unpushable anyway
# (.github/workflows/** needs a workflow-scoped token that
# automerge_pat deliberately lacks). Block below is byte-identical
# in lint.yml — the selftest drift-checks it; keep both in sync.
PR_FILES_LOG="$(mktemp)"
if ! gh api --paginate \
"repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/files" \
--jq '.[] | select(.status != "removed") | .filename' \
> "$PR_FILES_LOG" 2>/tmp/gh_err; then
echo "::warning::Could not list PR files (caller's GITHUB_TOKEN may lack 'pull-requests: read'). Falling back to full-glob mode."
LISTED=0
for attempt in 1 2 3; do
if gh api --paginate \
"repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/files" \
--jq '.[] | select(.status != "removed") | .filename' \
> "$PR_FILES_LOG" 2>/tmp/gh_err; then
LISTED=1
break
fi
echo "Changed-file listing attempt ${attempt}/3 failed:"
sed 's/^/ /' /tmp/gh_err || true
echo "mode=glob" >> "$GITHUB_OUTPUT"
if [ "$attempt" -lt 3 ]; then sleep "$(( attempt * 2 ))"; fi
done
if [ "$LISTED" -ne 1 ]; then
echo "::warning::Could not list PR files after 3 attempts (transient API error, or the caller's GITHUB_TOKEN lacks 'pull-requests: read'). Skipping prettier for this run — refusing to widen scope to the full tree."
echo "mode=none" >> "$GITHUB_OUTPUT"
exit 0
fi
mapfile -t CHANGED < "$PR_FILES_LOG"
Expand Down Expand Up @@ -292,6 +319,18 @@ jobs:
shell: bash
run: |
set -euo pipefail
# Never ship prettier writes under .github/workflows/: pushes
# touching workflow files need a workflow-scoped token, and
# automerge_pat deliberately lacks that scope (fleet policy) —
# GitHub rejects the whole push ("refusing to allow a Personal
# Access Token to create or update workflow ... without
# `workflow` scope"; whois-api-llc/wxa-secrets#27). Revert them:
# workflow-file formatting belongs in a human-pushed commit.
if ! git diff --quiet -- .github/workflows; then
echo "::notice::Reverting prettier write(s) under .github/workflows/ — automerge_pat has no workflow scope by design. Format workflow files in a human-pushed commit."
git diff --stat -- .github/workflows
git checkout -- .github/workflows
fi
if git diff --quiet; then
echo "::notice::No formatting changes — autofix is a no-op."
exit 0
Expand Down
7 changes: 7 additions & 0 deletions selftest/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,13 @@ arbitrary helper scripts; that's a different kind of repo.
- `test_prettier_symlink_filter.sh` — extracts the symlink filter from
`lint.yml` / `prettier-autofix.yml`, runs it against a fixture tree,
and asserts the two copies haven't drifted.
- `test_prettier_scope_failsafe.sh` — a failed changed-file listing must
SKIP the prettier run (mode=none), never fall open to the full-tree
glob; and prettier-autofix must revert writes under
`.github/workflows/` before committing (automerge_pat has no workflow
scope, so such pushes are always rejected — wxa-secrets#27). Extracts
and executes the shipped bash; drift-checks the listing block between
the two workflows.
- `test_workflow_guards.py` — pytest wrapper that runs the `.sh`
selftests above, so `tests-runner.yml`'s self-test path enforces them
in CI.
Expand Down
231 changes: 231 additions & 0 deletions selftest/test_prettier_scope_failsafe.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,231 @@
#!/usr/bin/env bash
# Behavioral test for prettier's changed-file scoping fail-safe in
# lint.yml / prettier-autofix.yml, and prettier-autofix.yml's
# workflow-file commit guard.
#
# Incident (whois-api-llc/wxa-secrets#27, run 29542573990): a transient
# GitHub API error (HTML error body made gh's --jq die with "invalid
# character '<' looking for beginning of value") failed the changed-file
# listing on a Python-only PR. The failure branch fell OPEN to full-glob
# mode: prettier --write ran on the entire tree, picked up pre-existing
# drift in .github/workflows/pr-review.yml — a file the PR never touched
# — and the push was rejected ("refusing to allow a Personal Access
# Token to create or update workflow ... without `workflow` scope").
# Red X on every such PR until the drift is manually cleared.
#
# Pins two invariants:
# 1. A failed changed-file listing SKIPS the run (mode=none). A
# scoping failure must never widen the target set to the full tree.
# 2. Autofix never commits writes under .github/workflows/ — the push
# PAT (automerge_pat) deliberately lacks workflow scope (fleet
# policy), so such a commit can never be pushed.
#
# Blocks are EXTRACTED from the workflow YAML and executed, so this
# exercises the shipped bash, not a mirrored copy. A drift check asserts
# lint.yml and prettier-autofix.yml carry an identical listing block.
#
# Run from the repo root:
# bash selftest/test_prettier_scope_failsafe.sh
set -euo pipefail

failed=0
T=$(mktemp -d)
trap 'rm -rf "$T"' EXIT

AUTOFIX=.github/workflows/prettier-autofix.yml
LINT=.github/workflows/lint.yml

# ---------------------------------------------------------------------------
# 1. Listing block: identical in both workflows; failure path sets
# mode=none and never mode=glob.
# ---------------------------------------------------------------------------

extract_listing_block() {
awk '/^[[:space:]]*PR_FILES_LOG="\$\(mktemp\)"/{grab=1} grab{print} grab && /mapfile -t CHANGED/{exit}' "$1"
}

a=$(extract_listing_block "$LINT")
b=$(extract_listing_block "$AUTOFIX")
if [ -z "$a" ] || [ -z "$b" ]; then
echo "✗ could not extract the changed-file listing block from one of the workflows"
exit 1
fi
if [ "$a" = "$b" ]; then
echo "✓ lint.yml and prettier-autofix.yml listing blocks are identical"
else
echo "✗ listing block drifted between lint.yml and prettier-autofix.yml:"
diff <(echo "$a") <(echo "$b") | sed 's/^/ /' || true
failed=1
fi

for wf in "$LINT" "$AUTOFIX"; do
blk=$(extract_listing_block "$wf")
if echo "$blk" | grep -q 'mode=glob'; then
echo "✗ ${wf}: listing-failure branch falls open to mode=glob (must skip with mode=none)"
failed=1
elif echo "$blk" | grep -q 'mode=none'; then
echo "✓ ${wf}: listing failure skips with mode=none"
else
echo "✗ ${wf}: listing-failure branch sets neither mode=none nor mode=glob"
failed=1
fi
done

# ---------------------------------------------------------------------------
# 2. Workflow-file commit guard in prettier-autofix.yml: present, and
# runs before `git add -A` stages the tree.
# ---------------------------------------------------------------------------

guard_line=$(grep -n 'git checkout -- \.github/workflows' "$AUTOFIX" | head -1 | cut -d: -f1 || true)
add_line=$(grep -n 'git add -A' "$AUTOFIX" | head -1 | cut -d: -f1 || true)
if [ -z "$guard_line" ]; then
echo "✗ prettier-autofix.yml: no workflow-file revert guard (git checkout -- .github/workflows)"
failed=1
elif [ -z "$add_line" ] || [ "$guard_line" -ge "$add_line" ]; then
echo "✗ prettier-autofix.yml: revert guard must run BEFORE git add -A"
failed=1
else
echo "✓ prettier-autofix.yml reverts .github/workflows/ writes before staging"
fi

extract_workflow_guard() {
awk '/if ! git diff --quiet -- \.github\/workflows/{grab=1} grab{print} grab && /^[[:space:]]*fi$/{exit}' "$AUTOFIX"
}

guard=$(extract_workflow_guard)
if [ -z "$guard" ]; then
echo "✗ could not extract the workflow-file revert guard block"
failed=1
else
snippet=$(echo "$guard" | sed 's/^[[:space:]]*//')
(
cd "$T"
git init -q fixture && cd fixture
git config user.email [email protected]
git config user.name selftest
mkdir -p .github/workflows
printf 'name: x\n' > .github/workflows/wf.yml
printf '# readme\n' > README.md
git add -A && git commit -qm init

printf 'name: x\nchanged: true\n' > .github/workflows/wf.yml
printf '# readme\nchanged\n' > README.md
eval "$snippet" > /dev/null
if git diff --quiet -- .github/workflows && ! git diff --quiet -- README.md; then
echo "✓ guard reverts workflow writes, keeps non-workflow writes"
else
echo "✗ guard: expected .github/workflows clean and README.md dirty"
exit 1
fi

git checkout -q -- README.md
printf 'name: x\nchanged: again\n' > .github/workflows/wf.yml
eval "$snippet" > /dev/null
if git diff --quiet; then
echo "✓ workflow-only drift becomes a full no-op"
else
echo "✗ guard: expected fully clean tree after workflow-only drift"
exit 1
fi
) || failed=1
fi

# ---------------------------------------------------------------------------
# 3. Behavioral scenarios with a stubbed `gh`.
# 3a runs the extracted listing block alone — its failure path exits
# before `mapfile`, so it works on any bash (incl. macOS 3.2).
# 3b/3c run the full resolve-targets script and need bash >= 4
# (mapfile, declare -A) — CI runners execute this with bash 5.
# ---------------------------------------------------------------------------

stub="$T/stub"; mkdir -p "$stub"
tree="$T/tree"; mkdir -p "$tree"
printf 'x = 1\n' > "$tree/app.py"
printf 'k: v\n' > "$tree/config.yml"
printf '# hi\n' > "$tree/README.md"

# 3a. Listing failure (the wxa-secrets#27 shape) → skip, not full-glob.
printf '#!/bin/sh\necho "gh: HTTP 502 from api.github.com" >&2\nexit 1\n' > "$stub/gh"
chmod +x "$stub/gh"
out="$T/out.failure"
: > "$out"
blk=$(extract_listing_block "$AUTOFIX")
(
cd "$tree" &&
env PATH="$stub:$PATH" \
GITHUB_OUTPUT="$out" GITHUB_REPOSITORY="o/r" \
GH_TOKEN=x PR_NUMBER=27 \
bash -c "set -euo pipefail
$blk" > "$out.log" 2>&1
) || true
if grep -q '^mode=none$' "$out" && ! grep -q 'mode=glob' "$out"; then
echo "✓ listing failure → mode=none (skip), never full-glob"
else
echo "✗ listing failure must set mode=none; GITHUB_OUTPUT was:"
sed 's/^/ /' "$out"
sed 's/^/ log: /' "$out.log"
failed=1
fi

if [ "${BASH_VERSINFO[0]}" -lt 4 ]; then
echo "– skipping full-script scenarios (bash ${BASH_VERSION%%(*} lacks mapfile; CI enforces them)"
else
extract_resolve_script() {
awk '
/- name: Resolve prettier targets/ {in_step=1}
in_step && /^ run: \|/ {in_run=1; next}
in_run {
if ($0 ~ /^ /) { print substr($0, 11); next }
if ($0 ~ /^[[:space:]]*$/) { print ""; next }
exit
}
' "$AUTOFIX"
}
script=$(extract_resolve_script)
if [ -z "$script" ]; then
echo "✗ could not extract the resolve-targets script from prettier-autofix.yml"
failed=1
else
scenario=0
run_scenario() { # $1 = gh stub body → prints path to GITHUB_OUTPUT file
scenario=$((scenario + 1))
local out="$T/out.$scenario"
: > "$out"
printf '#!/usr/bin/env bash\n%s\n' "$1" > "$stub/gh"
chmod +x "$stub/gh"
(
cd "$tree" &&
env PATH="$stub:$PATH" \
GITHUB_OUTPUT="$out" GITHUB_REPOSITORY="o/r" \
GH_TOKEN=x PR_NUMBER=27 CHANGED_ONLY=true \
GLOB='**/*.{md,yml,yaml,json}' \
bash -c "$script" > "$out.log" 2>&1
) || true
echo "$out"
}

# 3b. PR with zero prettier-relevant files (Python-only) → skip.
out=$(run_scenario 'printf "app.py\n"')
if grep -q '^mode=none$' "$out" && ! grep -q 'mode=glob' "$out"; then
echo "✓ python-only PR → mode=none (skip)"
else
echo "✗ python-only PR must set mode=none; GITHUB_OUTPUT was:"
sed 's/^/ /' "$out"
sed 's/^/ log: /' "$out.log"
failed=1
fi

# 3c. Positive control: a matching changed file → mode=files with it.
out=$(run_scenario 'printf "README.md\n"')
if grep -q '^mode=files$' "$out" && grep -q '^README\.md$' "$out"; then
echo "✓ matching changed file → mode=files targeting it"
else
echo "✗ expected mode=files with README.md; GITHUB_OUTPUT was:"
sed 's/^/ /' "$out"
sed 's/^/ log: /' "$out.log"
failed=1
fi
fi
fi

exit "$failed"
1 change: 1 addition & 0 deletions selftest/test_workflow_guards.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
"selftest/test_classify_bracket_guard.sh",
"selftest/test_classify_nocase.sh",
"selftest/test_pr_files_listing.sh",
"selftest/test_prettier_scope_failsafe.sh",
"selftest/test_prettier_symlink_filter.sh",
],
)
Expand Down
Loading