All checks were successful
CI / test (pull_request) Successful in 3m2s
CI / release-exercise (pull_request) Successful in 10s
CI / self-guards (pull_request) Successful in 6s
CI / action-exercise (pull_request) Successful in 6s
CI / docs-sync-exercise (pull_request) Successful in 6s
Refs guard / refs-not-closing (pull_request) Has been skipped
labels / labels (pull_request) Successful in 46s
@codex-reviewer-andresmgsl's second review, both points taken. The refs action goes back to `forge_preflight || exit 1`.97e63achad it exit 0 with a notice so the PR check would not be red, and that conflated two different questions: "this action cannot produce a verdict" is the ACTION's contract and must stay a refusal, while "this check should not block the board" is the CALLER's decision. The caller now carries it — refs-guard.yml skips unless github.server_url is github.com, mirroring forge_detect positively. A skipped check is a green head; an action that reports success it did not earn is not. The leaked preflight_err temp file goes with the revert. The workflow guard asked the wrong question. `command -v gh` alone passes the moment a Forgejo runner image happens to ship gh, and then dispatches against a forge that cannot serve it — the client/forge mismatch forge_preflight exists to prevent. It decides the FORGE first now, mirroring forge_detect positively, and the binary second. The source guard splits to match: a declaration guarded only by binary presence is reported, with a fixture that fails on exactly that shape. The warning text was also wrong on the facts, as noted: issue-event sweeps ARE this caller's event-driven wakes, so they are precisely what is lost. It now says the hourly scheduled sweep survives and every event-driven wake through this caller does not, until #205. Point 1 of that review — jq 1.6 accepting an empty payload — was already fixed in728102a, pushed before the review landed. Verified under the runner's jq 1.6 as well as 1.7: 28 test files, 0 failed both ways. shellcheck 0.10.0 (CI's pin), actionlint, self-ref, marker, vendored, changelog-armed all clean with every file tracked. Refs #198
232 lines
12 KiB
Bash
Executable file
232 lines
12 KiB
Bash
Executable file
#!/usr/bin/env bash
|
|
# The forge-portability guard (#198, enforcing #197's acceptance bar):
|
|
#
|
|
# No runtime `gh` invocation survives outside lib/forge-github.sh, except
|
|
# in a file that declares CEREMONY_FORGE_CLIENT=gh and therefore refuses
|
|
# loudly on a forge that cannot serve it.
|
|
#
|
|
# WHY THIS FILE EXISTS, rather than the rule living in review. #188 ported
|
|
# every `gh` call site onto the shim. The 0.6.0 upstream merge put SEVEN of
|
|
# them back — not in the eighteen conflict hunks, where a resolver would have
|
|
# been forced to look, but in whole functions upstream added to files this
|
|
# tree already owned. `git merge` takes upstream's side wherever only upstream
|
|
# moved a region, so it raised no conflict and asked no question. Reviewing
|
|
# the hunks could not have caught them; four reviewers reading the same diff
|
|
# each found a different subset.
|
|
#
|
|
# The sweep runs on a Forgejo instance whose runner image carries curl, jq and
|
|
# node and has NEITHER gh NOR stoke (lib/forge-forgejo.sh's header, probe task
|
|
# 278). So a reintroduced `gh` is not a style problem — it is `gh: command not
|
|
# found` mid-sweep, or a write that silently never happens.
|
|
#
|
|
# And it is invisible to the rest of the suite by construction: the contract
|
|
# tests stub `gh` as a shell function or on PATH, so they exercise a
|
|
# reintroduced call site happily and go green. This guard reads the SOURCE,
|
|
# which is the only place the difference is visible.
|
|
#
|
|
# It is deliberately a source-level check, and deliberately the ONLY one of
|
|
# its kind: every other guard here drives behaviour. This one cannot — the
|
|
# behaviour it forbids is unobservable in a harness that provides a `gh`.
|
|
set -u
|
|
|
|
ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
|
|
# shellcheck source=test/harness.sh
|
|
. "$ROOT/test/harness.sh"
|
|
|
|
# The backend that is ALLOWED to speak gh — it is the whole point of the file.
|
|
ALLOWED_FILE='lib/forge-github.sh'
|
|
|
|
|
|
# A file may opt out by declaring the client it speaks, which makes
|
|
# forge_preflight refuse by name on a forge that cannot serve it. Today that
|
|
# is actions/refs-not-closing, whose only gather is GraphQL and which Forgejo
|
|
# therefore cannot run at all (#199 ports it and drops the declaration).
|
|
# Both spellings, because both surfaces must be able to declare: `=` for a
|
|
# shell script, `:` for a workflow's env block. A filename exemption was the
|
|
# first shape here and @codex-reviewer-andresmgsl was right to reject it —
|
|
# it exempts the whole FILE, so any later gh call anywhere in that workflow
|
|
# would ride in free, and it lets a declaration exist without a refusal.
|
|
declares_gh_client() { grep -qE '^[[:space:]]*(export[[:space:]]+)?CEREMONY_FORGE_CLIENT[=:][[:space:]]*gh[[:space:]]*$' "$1"; }
|
|
|
|
# Declaring is half of it. #197's bar is "declared AND refuses loudly", and the
|
|
# refusal has TWO halves that a single check conflates
|
|
# (@codex-reviewer-andresmgsl, #198):
|
|
#
|
|
# * the FORGE — a gh dispatch is wrong on a forge that cannot serve it, and
|
|
# asking only "is gh installed?" passes the moment a Forgejo runner image
|
|
# happens to ship gh, which is the client/forge mismatch forge_preflight
|
|
# exists to prevent;
|
|
# * the BINARY — present or not on this runner.
|
|
#
|
|
# forge_preflight answers both, so a script that calls it satisfies both. A
|
|
# workflow has no shell to call it from and must do both inline.
|
|
#
|
|
# Comments are stripped first, for the same reason gh_calls strips them and
|
|
# with the same lesson learned the hard way: the first version of this
|
|
# predicate was satisfied by the word `forge_preflight` inside labels.yml's own
|
|
# comment EXPLAINING that it has no forge_preflight to call. A guard that reads
|
|
# prose as evidence is the blind sweep again, and it passed its own mutation
|
|
# test because of it.
|
|
strip_comments() { sed 's/[[:space:]]#.*$//; s/^[[:space:]]*#.*$//' "$1"; }
|
|
refuses_wrong_forge() {
|
|
strip_comments "$1" | grep -qE 'forge_preflight|GITHUB_SERVER_URL.*github\.com'
|
|
}
|
|
refuses_missing_binary() {
|
|
strip_comments "$1" | grep -qE 'forge_preflight|command -v gh'
|
|
}
|
|
refuses_when_unavailable() {
|
|
refuses_wrong_forge "$1" && refuses_missing_binary "$1"
|
|
}
|
|
|
|
# A runtime invocation, not the word. `gh` must be at a command position and
|
|
# followed by a gh subcommand — and comment lines are stripped first, because
|
|
# these surfaces document at length what gh used to do here and a guard that
|
|
# went red on its own prose would be deleted within a week
|
|
# (@kimi-reviewer-andresmgsl, #198). Nothing here reads a comment as evidence.
|
|
gh_calls() { # $1 = file → "line:code" per runtime gh invocation
|
|
# Comments are BLANKED rather than dropped, so grep -n still reports the
|
|
# file's real line numbers. Trailing comments go too, not just whole-line
|
|
# ones: a workflow's `actions: write # ...gh workflow run...` is prose
|
|
# about a call site, and YAML puts it after the code rather than before it.
|
|
sed 's/[[:space:]]#.*$//; s/^[[:space:]]*#.*$//' "$1" \
|
|
| grep -nE '(^|[^[:alnum:]_./$-])gh[[:space:]]+(api|issue|pr|release|repo|run|search|workflow|label|auth|browse|gist|secret|variable|ruleset)\b'
|
|
}
|
|
|
|
# The surfaces that run on a forge: executables and the workflows that call
|
|
# them. test/ is excluded on purpose — a test stubbing `gh` is the harness
|
|
# doing its job, and forbidding the string there would forbid the stubs that
|
|
# make the github backend testable at all.
|
|
scanned_files() {
|
|
local f
|
|
for f in "$ROOT"/lib/*.sh "$ROOT"/actions/*/*.sh "$ROOT"/bin/* \
|
|
"$ROOT"/.github/scripts/*.sh "$ROOT"/.github/workflows/*.yml; do
|
|
[ -f "$f" ] || continue
|
|
printf '%s\n' "${f#"$ROOT"/}"
|
|
done
|
|
}
|
|
|
|
offenders() {
|
|
local rel abs
|
|
while IFS= read -r rel; do
|
|
[ "$rel" = "$ALLOWED_FILE" ] && continue
|
|
abs="$ROOT/$rel"
|
|
if declares_gh_client "$abs"; then
|
|
refuses_when_unavailable "$abs" && continue
|
|
printf '%s: declares CEREMONY_FORGE_CLIENT=gh but carries no refusal\n' "$rel"
|
|
continue
|
|
fi
|
|
gh_calls "$abs" | sed "s|^|$rel:|"
|
|
done < <(scanned_files)
|
|
}
|
|
|
|
# In-process, not `bash -c`: a subshell cannot see these functions, so the
|
|
# sweep would find nothing, report empty, and pass by looking at nothing —
|
|
# the blind-sweep shape this guard exists to forbid, inside the guard itself.
|
|
no_offenders() {
|
|
local found
|
|
found="$(offenders)"
|
|
[ -z "$found" ] || { printf '%s\n' "$found" | sed 's/^/ /'; return 1; }
|
|
}
|
|
check "no runtime gh outside the github backend or a declared-client file" 0 "" \
|
|
no_offenders
|
|
|
|
# --- the guard has teeth ------------------------------------------------------
|
|
# A guard nobody has watched fail is a guard nobody is testing. These drive the
|
|
# predicates directly, because the sweep above is a property of the whole tree
|
|
# and cannot be made to fail without editing it.
|
|
|
|
TMP="$(mktemp -d)"
|
|
trap 'rm -rf "$TMP"' EXIT
|
|
|
|
# shellcheck disable=SC2016 # fixture CONTENT: the literal text a scanned file would hold
|
|
printf '%s\n' '#!/usr/bin/env bash' 'gh api "repos/$REPO/issues/1"' >"$TMP/bad.sh"
|
|
check "a reintroduced gh api read is seen" 0 "gh api" gh_calls "$TMP/bad.sh"
|
|
|
|
# shellcheck disable=SC2016 # fixture CONTENT: the literal text a scanned file would hold
|
|
printf '%s\n' '#!/usr/bin/env bash' 'run gh issue comment "$n" --body x' >"$TMP/bad2.sh"
|
|
check "a reintroduced gh issue write is seen, staged or not" 0 "gh issue" \
|
|
gh_calls "$TMP/bad2.sh"
|
|
|
|
# The exact shape the 0.6.0 merge reintroduced, indented inside a function.
|
|
# shellcheck disable=SC2016 # fixture CONTENT: the literal text a scanned file would hold
|
|
printf '%s\n' '#!/usr/bin/env bash' 'f() {' \
|
|
' guarded_read bodies gh api --paginate "repos/$REPO/issues/$1/comments"' '}' \
|
|
>"$TMP/bad3.sh"
|
|
check "...including one nested in a function behind guarded_read" 0 "gh api" \
|
|
gh_calls "$TMP/bad3.sh"
|
|
|
|
printf '%s\n' '#!/usr/bin/env bash' '# gh api used to live here (#188)' \
|
|
'# run gh issue comment — retired' >"$TMP/prose.sh"
|
|
check "prose about gh is not a call site" 1 "" gh_calls "$TMP/prose.sh"
|
|
|
|
# shellcheck disable=SC2016 # fixture CONTENT: the literal text a scanned file would hold
|
|
printf '%s\n' '#!/usr/bin/env bash' 'forge_api "repos/$REPO/issues/1"' \
|
|
'echo "the gh client speaks /api/v3"' >"$TMP/good.sh"
|
|
check "the shim verb is not mistaken for a call site" 1 "" gh_calls "$TMP/good.sh"
|
|
|
|
# Neighbouring identifiers must not read as the binary: `gh_calls`, `$gh`,
|
|
# a path ending in /gh, and `regh api` are all not an invocation of gh.
|
|
# shellcheck disable=SC2016 # fixture CONTENT: the literal text a scanned file would hold
|
|
printf '%s\n' '#!/usr/bin/env bash' 'gh_calls() { :; }' 'regh api foo' \
|
|
'echo "$gh api"' >"$TMP/lookalike.sh"
|
|
check "lookalike identifiers are not call sites" 1 "" gh_calls "$TMP/lookalike.sh"
|
|
|
|
printf '%s\n' '#!/usr/bin/env bash' 'export CEREMONY_FORGE_CLIENT=gh' \
|
|
'gh api graphql -f query=x' >"$TMP/declared.sh"
|
|
check "a declared-client file opts out" 0 "" declares_gh_client "$TMP/declared.sh"
|
|
# A workflow declares in YAML, not shell — both spellings must count, or the
|
|
# only surface that cannot call forge_preflight is also the only one that
|
|
# cannot declare.
|
|
printf '%s\n' 'jobs:' ' t:' ' steps:' ' - env:' \
|
|
' CEREMONY_FORGE_CLIENT: gh' ' run: gh workflow run x' \
|
|
>"$TMP/declared.yml"
|
|
check "...and so does a workflow declaring it in YAML" 0 "" \
|
|
declares_gh_client "$TMP/declared.yml"
|
|
# Declared is not enough: #197's bar is declared AND refuses loudly.
|
|
check "a declaration without a refusal is not enough" 1 "" \
|
|
refuses_when_unavailable "$TMP/declared.yml"
|
|
printf '%s\n' 'jobs:' ' t:' ' steps:' ' - env:' \
|
|
' CEREMONY_FORGE_CLIENT: gh' \
|
|
' run: |' \
|
|
' command -v gh >/dev/null || { echo "::warning::not woken"; exit 0; }' \
|
|
' gh workflow run x' >"$TMP/declared-refusing.yml"
|
|
# Binary presence ALONE is not a refusal: a Forgejo runner that ships gh would
|
|
# sail past it and dispatch against a forge that cannot serve the call.
|
|
check "...and a declaration guarded only by binary presence still is not" 1 "" \
|
|
refuses_when_unavailable "$TMP/declared-refusing.yml"
|
|
check "...though it does satisfy the binary half on its own" 0 "" \
|
|
refuses_missing_binary "$TMP/declared-refusing.yml"
|
|
# shellcheck disable=SC2016 # fixture CONTENT: the literal text a scanned file would hold
|
|
printf '%s\n' 'jobs:' ' t:' ' steps:' ' - env:' \
|
|
' CEREMONY_FORGE_CLIENT: gh' \
|
|
' run: |' \
|
|
' [ "$GITHUB_SERVER_URL" = "https://github.com" ] || exit 0' \
|
|
' command -v gh >/dev/null || exit 0' \
|
|
' gh workflow run x' >"$TMP/declared-both.yml"
|
|
check "...and a declaration guarding BOTH forge and binary is" 0 "" \
|
|
refuses_when_unavailable "$TMP/declared-both.yml"
|
|
# The shipped workflow is the real customer for that pair.
|
|
check "labels.yml declares the client it speaks" 0 "" \
|
|
declares_gh_client "$ROOT/.github/workflows/labels.yml"
|
|
check "...decides the forge before dispatching" 0 "" \
|
|
refuses_wrong_forge "$ROOT/.github/workflows/labels.yml"
|
|
check "...and checks the binary too, rather than dying on command not found" 0 "" \
|
|
refuses_missing_binary "$ROOT/.github/workflows/labels.yml"
|
|
check "...and an undeclared one does not" 1 "" declares_gh_client "$TMP/bad.sh"
|
|
# A mention of the variable in prose is not a declaration.
|
|
printf '%s\n' '#!/usr/bin/env bash' '# CEREMONY_FORGE_CLIENT=gh would opt out' \
|
|
>"$TMP/mentions.sh"
|
|
check "...nor does prose mentioning the variable" 1 "" \
|
|
declares_gh_client "$TMP/mentions.sh"
|
|
|
|
# The scan must actually reach the surfaces it claims to, or it passes by
|
|
# looking at nothing — the blind-sweep shape this repo keeps filing issues
|
|
# about, in its own guard.
|
|
scan_is_wide() { [ "$(scanned_files | wc -l)" -ge 20 ]; }
|
|
check "the scan reaches every executable surface" 0 "" scan_is_wide
|
|
scan_lists_backend() { scanned_files | grep -F lib/forge-github.sh; }
|
|
check "...including the backend it exempts" 0 "lib/forge-github.sh" scan_lists_backend
|
|
check "...and the backend really does speak gh, so the exemption is load-bearing" 0 "gh api" \
|
|
gh_calls "$ROOT/lib/forge-github.sh"
|
|
|
|
summary
|