Skip to content

Commit 81dbf2d

Browse files
fix(ci): confirm deny-list negatives too, and fix a cwd-relative shallow probe (#524)
Follow-up to #522, which merged before this landed. **#522 is live on `main` now, so these two defects are live on ~300 consumers.** Both were found by adding a test for the path production actually takes. Cases 1–12 of the fixture suite run against a **complete** `git init` repo; CI runs against a **partial** clone (`--filter=tree:0`). Nothing tested the code that protects production, and it had two bugs. ## 1. The deny-list's *negative* still trusted local `merge-base` on a partial clone A treeless clone resolves every commit, so the API fallback was gated on "commits do not resolve" — making it **dead code in CI**. Trace `awesome-haskell` (pin `5a93d9d5`, which predates the Hypatia fix and *must* be denied): ``` pin_is_known_bad -> local branch taken -> wrong negative -> continue -> NOT denied classify_pin -> same mis-fire -> API says ahead=90 -> OUT_OF_WINDOW -> notice -> PASS, while running the frozen scanner ``` That is the exact false green the deny-list exists to prevent, on the very cohort that proved `merge-base` is unreliable there. **Fix:** a local **positive** is still trusted (no false positives observed, and a deny must not require the network), but a local **negative** is accepted only from a *complete* clone — otherwise it is confirmed server-side. ## 2. `clone_is_complete` used a cwd-relative git-dir `rev-parse --git-dir` returns `.git`, and the check was `[ -e "$gd/shallow" ]` — which resolves against the **CWD**, i.e. the *consumer's* checkout, which `actions/checkout` makes shallow by default. So `.git/shallow` would nearly always exist and **every** standards clone would be classified incomplete, permanently masking the complete-clone fast path. Now uses `--absolute-git-dir`. Test 13c failed for exactly this reason, which is how it was found. ## 3. `governance-reusable.yml` passes `GITHUB_TOKEN` to the staleness step The server-side confirmation is unauthenticated — 60/hr per runner IP — and exhausting it degrades the gate to `::warning`. That fail-open is deliberate and **loud** (two warnings naming the pin; never a silent pass), and the common path never calls the API at all, since a genuine deny is a local positive. A token removes the cliff. This reaches a consumer only once it re-pins past this commit — the script is read from `standards` HEAD, but the job definition comes from the pinned SHA — so it lands naturally with the propagation the deny-listed pins need anyway. ## Tests: 16 → 19, still hermetic The API is pointed at a closed port (`http://127.0.0.1:1`), so these are fast and offline: ``` PASS: partial clone: local positive still denies without the network PASS: partial clone: unverifiable deny-list check is reported SKIPPED, not passed PASS: complete clone: local negative is trusted silently (no SKIPPED warning) ``` `run_case_out` gained a leading-`!` inversion so a test can assert the gate did **not** say something. Suite verified from two different working directories, since defect (2) was cwd-dependent. 19/19 pass on top of current `main`; `shellcheck -S warning` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent d13ff9f commit 81dbf2d

4 files changed

Lines changed: 97 additions & 5 deletions

File tree

.github/workflows/governance-reusable.yml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,16 @@ jobs:
4444
|| git clone --depth 200 https://github.com/hyperpolymath/standards.git "$HOME/standards"
4545
4646
- name: Run staleness check
47+
env:
48+
# The gate confirms a pin server-side before ever accusing it of being
49+
# forged, and before accepting a deny-list negative from a clone it
50+
# cannot trust. Unauthenticated that is 60 requests/hour per runner IP;
51+
# exhausting it makes the gate degrade to ::warning (loudly — it never
52+
# silently passes, see check-workflow-staleness.sh). A token removes
53+
# the cliff entirely. Note this only reaches a consumer once it re-pins
54+
# past this commit: the script comes from standards HEAD, but the job
55+
# definition comes from whatever SHA the consumer pins.
56+
GITHUB_TOKEN: ${{ github.token }}
4757
run: |
4858
bash "$HOME/standards/scripts/check-workflow-staleness.sh" .
4959

docs/decisions/ADR-003-workflow-pin-staleness-window.adoc

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,3 +223,23 @@ Cost is zero API calls on the happy path — the server is consulted only when
223223
the gate is about to accuse. The deny-list gets the same fallback, so a
224224
degraded runner cannot silently skip it: "could not check" is reported as
225225
SKIPPED, never as passed.
226+
227+
==== Rate limiting, and why it is not a hole
228+
229+
The staleness job is given no token, so the compare call is unauthenticated: 60
230+
requests/hour per runner IP. Exhausting it makes the gate emit `::warning` and
231+
pass — verified by accident while testing, which is exactly the intended
232+
fail-open, and it is *loud*: two warnings naming the pin, never a silent pass.
233+
234+
Three things keep this from mattering:
235+
236+
. The common path never calls the API. A pin that is genuinely deny-listed is
237+
caught by a local *positive*, which is trusted without the network — so the
238+
294 `d7c22711` denials do not depend on it at all.
239+
. The API is consulted only when the gate is about to accuse, or when a
240+
negative comes from a clone it cannot trust.
241+
. `governance-reusable.yml` now passes `GITHUB_TOKEN: ${{ github.token }}` to
242+
the step, which removes the limit. This reaches a consumer only once it
243+
re-pins past that commit — the script is read from `standards` HEAD, but the
244+
job definition comes from the pinned SHA — so it lands naturally with the
245+
propagation that these deny-listed pins need anyway.

scripts/check-workflow-staleness.sh

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ set -eo pipefail
6767
# server-side integrity check (default
6868
# hyperpolymath/standards).
6969
# STALENESS_STANDARDS_BRANCH— branch a pin must be reachable from (default main).
70+
# STALENESS_API_BASE — API root for the server-side integrity check
71+
# (default https://api.github.com; set for GHES,
72+
# or to an unreachable host in tests).
7073
# STALENESS_KNOWN_BAD_BEFORE— override the known-bad deny-list, as space- or
7174
# comma-separated <reusable>:<fix-sha> entries.
7275
# "-" disables it. For the hermetic fixture tests
@@ -82,6 +85,7 @@ WINDOW_DAYS="${STALENESS_WINDOW_DAYS:-14}"
8285
# below). Overridable so a fork can point the gate at its own standards.
8386
STANDARDS_NWO="${STALENESS_STANDARDS_NWO:-hyperpolymath/standards}"
8487
STANDARDS_BRANCH="${STALENESS_STANDARDS_BRANCH:-main}"
88+
STANDARDS_API="${STALENESS_API_BASE:-https://api.github.com}"
8589

8690
# ── Known-bad pins: hard fail regardless of age ─────────────────────────────
8791
# A recency *window* is only a proxy for "this pin might contain a known
@@ -213,8 +217,12 @@ ensure_commit() {
213217
clone_is_complete() {
214218
[ "$HAVE_HISTORY" = true ] || return 1
215219
local gd
216-
gd=$(git -C "$STANDARDS_DIR" rev-parse --git-dir 2>/dev/null) || return 1
217-
[ -e "$STANDARDS_DIR/$gd/shallow" ] || [ -e "$gd/shallow" ] && return 1
220+
# --absolute-git-dir, not --git-dir: the latter returns a path relative to the
221+
# CWD, which here is the *consumer's* checkout. actions/checkout is shallow by
222+
# default, so a relative ".git/shallow" test would see the consumer's marker
223+
# and call every standards clone incomplete.
224+
gd=$(git -C "$STANDARDS_DIR" rev-parse --absolute-git-dir 2>/dev/null) || return 1
225+
[ -e "$gd/shallow" ] && return 1
218226
[ -n "$(git -C "$STANDARDS_DIR" config --get remote.origin.partialclonefilter 2>/dev/null)" ] && return 1
219227
[ "$(git -C "$STANDARDS_DIR" config --get remote.origin.promisor 2>/dev/null)" = "true" ] && return 1
220228
return 0
@@ -235,7 +243,7 @@ resolve_negative() {
235243
api_compare() {
236244
local pin="$1" head="${2:-$STANDARDS_BRANCH}" url body code st ahead behind
237245
command -v curl >/dev/null 2>&1 || return 1
238-
url="https://api.github.com/repos/${STANDARDS_NWO}/compare/${pin}...${head}"
246+
url="${STANDARDS_API}/repos/${STANDARDS_NWO}/compare/${pin}...${head}"
239247
local -a auth=()
240248
[ -n "${GITHUB_TOKEN:-}" ] && auth=(-H "Authorization: Bearer ${GITHUB_TOKEN}")
241249
body=$(curl -sS --max-time 20 -w '\n%{http_code}' \
@@ -298,7 +306,13 @@ pin_is_known_bad() {
298306
KNOWN_BAD_FIX="$fix_full"
299307
return 0
300308
fi
301-
continue
309+
# Local says "not affected". A local POSITIVE is trustworthy (no false
310+
# positives observed), but a local NEGATIVE here is a FALSE GREEN — the
311+
# exact failure this list exists to prevent — and the same
312+
# `merge-base --is-ancestor` call is measured unreliable on the partial
313+
# clone CI actually uses. Trust the negative only from a complete clone;
314+
# otherwise fall through and confirm with the server.
315+
clone_is_complete && continue
302316
fi
303317

304318
# Degraded clone: ask the server rather than skip. A check that quietly

scripts/tests/check-workflow-staleness-test.sh

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,12 @@ run_case_out() {
9191
bash "$CHECK_SCRIPT" "$repo" 2>&1)
9292
rc=$?
9393
set -e
94-
if [ "$rc" -eq "$expected" ] && printf '%s' "$out" | grep -qE "$regex"; then
94+
# A leading '!' inverts the match: assert the gate did NOT say this.
95+
local want=0
96+
case "$regex" in '!'*) want=1; regex="${regex#!}" ;; esac
97+
local got=0
98+
printf '%s' "$out" | grep -qE "$regex" || got=1
99+
if [ "$rc" -eq "$expected" ] && [ "$got" -eq "$want" ]; then
95100
echo "PASS: $desc"
96101
PASS=$((PASS + 1))
97102
else
@@ -214,6 +219,49 @@ write_pin "$R/.github/workflows/hypatia.yml" "hypatia-scan-reusable.yml" "$C4"
214219
write_pin "$R/.github/workflows/scorecard.yml" "scorecard-reusable.yml" "$C4"
215220
run_case "clean multi-reusable repo passes" 0 "$R"
216221

222+
# ── 13. THE PRODUCTION PATH: partial clone, where a local negative is not
223+
# trustworthy ──────────────────────────────────────────────────────────
224+
# In CI the governance reusable clones standards with `--filter=tree:0`, so the
225+
# checkout is ALWAYS partial — and `merge-base --is-ancestor` is measured
226+
# unreliable there (awesome-haskell, four consecutive runs). Cases 1-12 all run
227+
# against a complete `git init` fixture, so none of them exercise the code that
228+
# actually protects production. Mark the fixture partial to reach it.
229+
#
230+
# The API is pointed at a closed port so these stay hermetic and fast: the
231+
# question is what the gate does when it can neither trust the clone nor reach
232+
# the server.
233+
UNREACHABLE="http://127.0.0.1:1"
234+
git -C "$FIX" config remote.origin.promisor true
235+
236+
# 13a. A local POSITIVE is still trusted — a deny must not need the network.
237+
R="$TEST_DIR/partial-deny"; mk_repo "$R"
238+
write_pin "$R/.github/workflows/governance.yml" "governance-reusable.yml" "$C1"
239+
run_case_out "partial clone: local positive still denies without the network" 1 \
240+
'predates' "$R" \
241+
STALENESS_WINDOW_COMMITS=9999 STALENESS_WINDOW_DAYS=9999 \
242+
STALENESS_API_BASE="$UNREACHABLE" \
243+
STALENESS_KNOWN_BAD_BEFORE="governance-reusable.yml:$C3"
244+
245+
# 13b. A local NEGATIVE must NOT be silently accepted. Unverifiable is reported
246+
# as SKIPPED — "could not check" must never read as "passed".
247+
R="$TEST_DIR/partial-unchecked"; mk_repo "$R"
248+
write_pin "$R/.github/workflows/governance.yml" "governance-reusable.yml" "$C4"
249+
run_case_out "partial clone: unverifiable deny-list check is reported SKIPPED, not passed" 0 \
250+
'SKIPPED, not passed' "$R" \
251+
STALENESS_WINDOW_COMMITS=9999 STALENESS_WINDOW_DAYS=9999 \
252+
STALENESS_API_BASE="$UNREACHABLE" \
253+
STALENESS_KNOWN_BAD_BEFORE="governance-reusable.yml:$C3"
254+
255+
# 13c. A complete clone's negative IS trusted — no warning, no network.
256+
git -C "$FIX" config --unset remote.origin.promisor
257+
R="$TEST_DIR/complete-quiet"; mk_repo "$R"
258+
write_pin "$R/.github/workflows/governance.yml" "governance-reusable.yml" "$C4"
259+
run_case_out "complete clone: local negative is trusted silently (no SKIPPED warning)" 0 \
260+
'!SKIPPED' "$R" \
261+
STALENESS_WINDOW_COMMITS=9999 STALENESS_WINDOW_DAYS=9999 \
262+
STALENESS_API_BASE="$UNREACHABLE" \
263+
STALENESS_KNOWN_BAD_BEFORE="governance-reusable.yml:$C3"
264+
217265
echo "----------------------------------------"
218266
echo "$PASS/$TOTAL test cases passed."
219267
[ "$PASS" -eq "$TOTAL" ] || { echo "Some regression tests FAILED."; exit 1; }

0 commit comments

Comments
 (0)