From c11a3307049a6b29a2929ddd7806203527998d78 Mon Sep 17 00:00:00 2001 From: topcoder1 Date: Sat, 17 Oct 2026 11:57:04 -0700 Subject: [PATCH 1/7] test(audit-required-checks): every rules page, and encoded branch and file names (RED) The validator's rules call reads one page of the branch's rules (30 a page), so a required_status_checks rule past page 1 reads as no required checks, exit 0. --branch and workflow file names go into the URL unencoded: `#` starts a fragment gh never sends, `?` starts the query, and a bad `%` fails inside gh. These cases fail on the current validator (15 of 65 checks, the same on bash 3.2 and bash 5); the fix follows. The shim now reads an endpoint the way gh 2.97.0 and GitHub did (measured 2026-10-16 on topcoder1/dotclaude, and 2026-10-17 on this repo) and pages the rules answer; shim controls s1-s4 pin that. Co-Authored-By: Claude Opus 5.5 --- selftest/test_audit_required_checks.sh | 423 ++++++++++++++++++++++++- 1 file changed, 414 insertions(+), 9 deletions(-) diff --git a/selftest/test_audit_required_checks.sh b/selftest/test_audit_required_checks.sh index 00b6f86..2dc637e 100644 --- a/selftest/test_audit_required_checks.sh +++ b/selftest/test_audit_required_checks.sh @@ -50,12 +50,30 @@ # `==> FAIL` verdict, so an unbound variable or a SyntaxError (both exit # 1) is an ERROR; and malformed answers reach the digest, a PUBLIC issue, # as API errors carrying none of their bytes +# h the rules call reads every page (2026-10-17): a required_status_checks +# rule as rule 31 (past the 30 an unpaginated call read) or rule 101 +# (past a page of 100) is validated, in both forms gh --paginate prints; +# a bad rule on page 1 of 2, a page 2 that is not a list, and a page 2 +# that fails each exit 3, echoing none of the answer +# i --branch and workflow file names go into the URL percent-encoded, `/` +# kept (2026-10-17): a file name with `#`, `?`, `%` or a space is read as +# that file, a branch with `#` or `%41` is read as that branch, and an +# API error names the endpoint as it was sent # Negative controls (the shim reproduces each old failure, run verbatim): # n1 the old rules fetch + filter dies with jq exit 5 on case a's 401 # n4 the old rules fetch reads case a's no-response failure as "no branch # rules found" and exits 0: a repo it never reached passes as clean # n2 the old listing fetch reads case b's 403 as zero workflow names # n3 the old per-file fetch turns case b's 404 into an empty workflow file +# Shim controls: cases h and i tell the fix from the bug only while the shim +# reads a request as gh and GitHub do, so these pin it on the requests the +# validator sent before them (the rules call without --paginate, each value +# as it was given) and on both forms --paginate prints: +# s1 the rules answer comes 30 rules a page, and per_page and page select +# s2 --paginate prints every page, as one array or as an array per page +# s3 explicit page bytes print as given, a failing page included +# s4 an unencoded `#`, `?` or bad `%` reads another file or fails in gh, +# and an unencoded branch reads another branch's rules # # Run from the repo root: # bash selftest/test_audit_required_checks.sh @@ -81,12 +99,32 @@ show() { cat -- "$@" 2>/dev/null || true; } # --------------------------------------------------------------------------- # The shim. One fake repo per directory; `fail` holds " " lines -# ( is the endpoint after /repos///), `listing.json`, when -# present, replaces the generated directory listing, and `answers/` -# replaces a workflow file's contents answer. Every call is logged. Like gh, -# it applies a --jq filter itself, and a filter that fails prints part of the -# response in its error (measured 2026-10-16, gh 2.97.0: +# ( is the endpoint after /repos/// in the request gh +# sends, query string included), `listing.json`, when present, replaces the +# generated directory listing, and `answers/` replaces a workflow file's +# contents answer. With a `branch-rules/` directory, each branch's rules are +# in branch-rules//rules.json and any other branch has none; +# `pages_apart` prints a --paginate rules answer as an array per page; and +# `rule-pages/.json` holds the bytes gh prints for page of the rules +# answer, or `.fail` the status that page fails with. Every call is +# logged. Like gh, it applies a --jq filter itself, and a filter that fails +# prints part of the response in its error (measured 2026-10-16, gh 2.97.0: # `expected an object but got: array ([{"_links":{"git":"https: ...])`). +# +# It reads an endpoint the way gh 2.97.0 and GitHub did, measured 2026-10-16 +# on topcoder1/dotclaude and against a local server, and 2026-10-17 on this +# repo. gh parses the endpoint as a URL. It never sends a #fragment +# (`rules/branches/main#x` read main's rules), and a % in the path that starts +# no %XX escape fails in gh before anything is sent (`invalid URL escape +# "%zz"`, rc 1). GitHub decodes %XX in the path (`rules/branches/ma%69n` read +# main's rules) and in the query, where `&` starts the next parameter and a +# parameter with a bad % is dropped. It pages the rules answer, 30 rules a +# page unless per_page asks for up to 100. gh --paginate asks for 100 unless +# the endpoint names a per_page, follows every next page, and prints the pages +# of an array as one array, the `]` only after the last page (a null page 2 +# printed `[{"type":"deletion"}null`, rc 0; a 403 on page 2 printed page 1, +# then the error body, rc 1). gh's help instead describes an array per page, +# the form `pages_apart` gives. # --------------------------------------------------------------------------- mkdir -p "$T/bin" "$T/gh" cat > "$T/bin/gh" <<'STUB' @@ -97,7 +135,13 @@ if [ "${1:-}" != api ]; then echo "shim: unexpected gh $*" >&2 exit 98 fi -ep="${2#/}" +url="${2%%#*}" +bad_escape='%($|[^0-9A-Fa-f]|[0-9A-Fa-f]($|[^0-9A-Fa-f]))' +if grep -Eq "$bad_escape" <<< "${url%%\?*}"; then + echo "parse \"https://api.github.com/${url#/}\": invalid URL escape" >&2 + exit 1 +fi +ep="${url#/}" ep="${ep#repos/}" owner="${ep%%/*}" ep="${ep#*/}" @@ -105,11 +149,35 @@ repo="${ep%%/*}" rest="${ep#*/}" dir="$GH_FAKE/$owner/$repo" filter="" +paginate=no prev="" for a in "$@"; do if [ "$prev" = "--jq" ]; then filter="$a"; fi + if [ "$a" = --paginate ]; then paginate=yes; fi prev="$a" done +pct_decode() { printf '%b' "${1//%/\\x}"; } +path=$(pct_decode "${rest%%\?*}") +per_page="" page=1 +case "$rest" in + *\?*) query="${rest#*\?}" ;; + *) query="" ;; +esac +while [ -n "$query" ]; do + param="${query%%&*}" + case "$query" in + *'&'*) query="${query#*&}" ;; + *) query="" ;; + esac + if grep -Eq "$bad_escape" <<< "$param"; then continue; fi + case "$param" in + per_page=*) per_page=$(pct_decode "${param#*=}") ;; + page=*) page=$(pct_decode "${param#*=}") ;; + esac +done +if [ -z "$per_page" ]; then + if [ "$paginate" = yes ]; then per_page=100; else per_page=30; fi +fi respond() { # the answer on stdin -> stdout, through --jq when one was given local body body=$(cat) @@ -147,9 +215,56 @@ if [ -f "$dir/fail" ]; then done < "$dir/fail" fi [ -d "$dir" ] || fail_with 404 -case "$rest" in +serve_rules() { # rules file -> the rules answer, paged + local n from total + if [ -d "$dir/rule-pages" ]; then + # Page after page, as given; with --paginate, up to the first page that + # fails, which ends the call with that page's error. + n="$page" + while :; do + if [ -f "$dir/rule-pages/$n.fail" ]; then + fail_with "$(cat "$dir/rule-pages/$n.fail")" + fi + cat "$dir/rule-pages/$n.json" + n=$((n + 1)) + if [ "$paginate" = no ]; then break; fi + if [ ! -f "$dir/rule-pages/$n.json" ] && [ ! -f "$dir/rule-pages/$n.fail" ]; then + break + fi + done + return + fi + if ! jq -e 'type == "array"' "$1" > /dev/null 2>&1; then + cat "$1" # not a list: nothing to page + return + fi + case "$per_page" in '' | *[!0-9]*) per_page=30 ;; esac + case "$page" in '' | *[!0-9]*) page=1 ;; esac + if [ "$per_page" -gt 100 ]; then per_page=100; fi + from=$(((page - 1) * per_page)) + if [ "$paginate" = no ]; then + jq -c --argjson from "$from" --argjson n "$per_page" '.[$from:$from + $n]' "$1" + elif [ -f "$dir/pages_apart" ]; then + total=$(jq length "$1") + while :; do + jq -jc --argjson from "$from" --argjson n "$per_page" '.[$from:$from + $n]' "$1" + from=$((from + per_page)) + if [ "$from" -ge "$total" ]; then break; fi + done + else + jq -c --argjson from "$from" '.[$from:]' "$1" + fi +} +case "$path" in rules/branches/*) - respond < "$dir/rules.json" ;; + branch="${path#rules/branches/}" + if [ ! -d "$dir/branch-rules" ]; then + serve_rules "$dir/rules.json" + elif [ -f "$dir/branch-rules/$branch/rules.json" ]; then + serve_rules "$dir/branch-rules/$branch/rules.json" + else + echo '[]' + fi ;; contents) echo '[{"name":"README.md","type":"file"}]' | respond ;; contents/.github/workflows) @@ -160,7 +275,7 @@ case "$rest" in (cd "$dir/workflows" && ls) | jq -R . | jq -s 'map({name: ., type: "file"})' | respond fi ;; contents/.github/workflows/*) - name="${rest#contents/.github/workflows/}" + name="${path#contents/.github/workflows/}" if [ -f "$dir/answers/$name" ]; then respond < "$dir/answers/$name" else @@ -196,6 +311,18 @@ rules_json() { # required contexts... -> a rules answer in the API's shape for c in "$@"; do ctx="$ctx${ctx:+,}{\"context\":\"$c\"}"; done printf '[{"type":"deletion","ruleset_source_type":"Repository","ruleset_source":"o/r","ruleset_id":1},{"type":"required_status_checks","parameters":{"strict_required_status_checks_policy":false,"do_not_enforce_on_create":false,"required_status_checks":[%s]},"ruleset_source_type":"Repository","ruleset_source":"o/r","ruleset_id":1}]\n' "$ctx" } +rules_after() { # count, required contexts... -> a rules answer: count rules + # that require no check, then a required_status_checks rule listing the + # contexts, rule count + 1 + jq -nc --argjson n "$1" ' + [range(1; $n + 1) + | {type: "deletion", ruleset_source_type: "Repository", ruleset_source: "o/r", ruleset_id: .}] + + [{type: "required_status_checks", + parameters: {strict_required_status_checks_policy: false, do_not_enforce_on_create: false, + required_status_checks: [$ARGS.positional[] | {context: .}]}, + ruleset_source_type: "Repository", ruleset_source: "o/r", ruleset_id: ($n + 1)}]' \ + --args "${@:2}" +} fake_repo() { # owner/repo, rules answer, workflow fixtures... local d="$T/gh/$1" w mkdir -p "$d/workflows" @@ -695,6 +822,185 @@ else grep -n "PRIVATE-BODY-CANARY" "$T/digest.md" "$T/f.err" || true fi +# --------------------------------------------------------------------------- +# h. The rules call reads every page. The endpoint pages its answer, 30 rules +# a page unless per_page asks for up to 100, and the required_status_checks +# rule can be on any page. Read without --paginate, one past page 1 was +# never seen, and the branch passed as having no required checks, exit 0, +# even with a required context that has no producer. Each case runs in +# both forms --paginate output takes: gh 2.97.0 prints the pages of an +# array as one array, and gh's help describes an array per page. +# --------------------------------------------------------------------------- +# Rule 31 is past the 30 rules an unpaginated call reads; rule 101 is past +# the first page of 100 that --paginate asks for. +for rule_no in 31 101; do + for form in one-array array-per-page; do + r="topcoder1/rule$rule_no-$form" + fake_repo "$r" "$(rules_after $((rule_no - 1)) test "plan-proofs / verify")" ci.yml + if [[ $form == array-per-page ]]; then touch "$T/gh/$r/pages_apart"; fi + run_validator "$r" + if [[ $VRC -eq 1 ]] && has "$T/v.out" "found 2 required context(s):" && + has "$T/v.out" "✓ test" && has "$T/v.out" "✗ plan-proofs / verify"; then + pass "h: a required check rule as rule $rule_no is read and validated ($form)" + else + fail "h: rule $rule_no ($form) should be read and its phantom found, exit 1 (got $VRC)" + show "$T/v.out" "$T/v.err" + fi + done +done + +# Printed as an array per page, the answer is several JSON values, and jq +# goes by the last one only, in `jq -e` and in its exit status alike (jq +# 1.7.1, measured 2026-10-17: an error on an earlier input still exits 0). +# Unmerged, a rule that is not an object on page 1 of 2 read as a clean pass; +# it must fail the answer as it does when there is one page. +fake_repo topcoder1/badpage "$(rules_after 100 test | jq -c '.[0] = "PRIVATE-BODY-CANARY-badpage"')" ci.yml +touch "$T/gh/topcoder1/badpage/pages_apart" +run_validator topcoder1/badpage +if [[ $VRC -eq 3 ]] && + has "$T/v.err" "API error: GET /repos/topcoder1/badpage/rules/branches/main did not answer an array of rules" && + api_error_only && no_body_bytes; then + pass "h: a bad rule on page 1 of 2 exits 3 and echoes none of the answer" +else + fail "h: a bad rule on page 1 of 2 should exit 3 (got $VRC)" + show "$T/v.out" "$T/v.err" +fi + +rule_pages() { # owner/repo, page-1 form, page 1's rules, page 2's bytes or HTTP status + # Page 1 as gh prints it: an array of its own, or (gh 2.97.0) the array + # left open for the pages after it. + local d="$T/gh/$1/rule-pages" + mkdir -p "$d" + if [[ $2 == gh-2.97.0 ]]; then + printf '%s' "${3%]}" > "$d/1.json" + else + printf '%s' "$3" > "$d/1.json" + fi + case "$4" in + [0-9][0-9][0-9]) echo "$4" > "$d/2.fail" ;; + *) printf '%s' "$4" > "$d/2.json" ;; + esac +} +page1=$(rules_after 29 "plan-proofs / verify" | sed 's#"o/r"#"PRIVATE-BODY-CANARY-page1"#g') +for form in array-per-page gh-2.97.0; do + # A null page 2. An array per page prints `[page 1]null`, and jq's `add` + # takes a null for nothing at all: merged, it is page 1 alone, judged as if + # it held every rule, and page 1's phantom would exit 1. gh 2.97.0 prints + # `[page 1's rules null`, which is not JSON at all. + r="topcoder1/nullpage-$form" + fake_repo "$r" '[]' ci.yml + rule_pages "$r" "$form" "$page1" null + run_validator "$r" + if [[ $VRC -eq 3 ]] && + has "$T/v.err" "API error: GET /repos/$r/rules/branches/main did not answer an array of rules" && + api_error_only && no_body_bytes; then + pass "h: a page 2 that is not a list exits 3 and echoes none of the answer ($form)" + else + fail "h: a null page 2 ($form) should exit 3 (got $VRC)" + show "$T/v.out" "$T/v.err" + fi + + # A page 2 that fails: gh --paginate exits 1 there, after printing the + # pages before it, and page 1's rules must never be judged on their own. + r="topcoder1/failpage-$form" + fake_repo "$r" '[]' ci.yml + rule_pages "$r" "$form" "$(rules_after 29 test)" 403 + run_validator "$r" + if [[ $VRC -eq 3 ]] && + has "$T/v.err" "API error: GET /repos/$r/rules/branches/main failed (HTTP 403)" && + has "$T/v.err" "gh: Resource not accessible by personal access token (HTTP 403)" && + api_error_only; then + pass "h: a page 2 that fails exits 3 naming its status ($form)" + else + fail "h: a page 2 that fails ($form) should exit 3 with HTTP 403 (got $VRC)" + show "$T/v.out" "$T/v.err" + fi +done + +# --------------------------------------------------------------------------- +# i. Values in the URL are percent-encoded. gh parses the endpoint as a URL, +# so a value spliced in as it is can read something else: `#` starts a +# fragment gh never sends, `?` starts the query, and a `%` that starts no +# %XX escape stops gh before it sends anything, while GitHub decodes any +# %XX that is left. A `/` is sent as it is. +# --------------------------------------------------------------------------- +# Each file is its repo's only producer of the required check. Unencoded, +# `#` and `?` cut the name to `ci`, a file the repo does not have, and `%.` +# stopped gh. A space always worked: gh escapes it itself. +n=0 +for name in "ci#2.yml" "ci?2.yml" "50%.yml" "ci 2.yml"; do + n=$((n + 1)) + r="topcoder1/filename$n" + fake_repo "$r" "$(rules_json test)" + cp "$T/wf/ci.yml" "$T/gh/$r/workflows/$name" + run_validator "$r" + if [[ $VRC -eq 0 ]] && has "$T/v.out" "✓ test" && has "$T/v.out" "target/$name (direct match"; then + pass "i: a workflow file named '$name' is read as that file" + else + fail "i: the file '$name' should be read as the producer of test, exit 0 (got $VRC)" + show "$T/v.out" "$T/v.err" + fi +done + +# Only the named branch has rules. Unencoded, `#` cut it to `release` and +# GitHub decoded %41 to `A`: branches with no rules, which read as a clean +# pass. +fake_repo topcoder1/branches '[]' ci.yml +for branch in "release#2" "rel%41se"; do + mkdir -p "$T/gh/topcoder1/branches/branch-rules/$branch" + rules_json test "plan-proofs / verify" > "$T/gh/topcoder1/branches/branch-rules/$branch/rules.json" +done +for branch in "release#2" "rel%41se"; do + run_validator topcoder1/branches --branch "$branch" + if [[ $VRC -eq 1 ]] && has "$T/v.out" "(branch: $branch)" && + has "$T/v.out" "✓ test" && has "$T/v.out" "✗ plan-proofs / verify"; then + pass "i: --branch '$branch' reads that branch's rules" + else + fail "i: --branch '$branch' should read its own rules and find the phantom, exit 1 (got $VRC)" + show "$T/v.out" "$T/v.err" + fi +done + +# An API error names the endpoint as it was sent: a file read that fails, a +# file answer that is not base64 content, and a rules answer that is not a +# list, for a branch whose `/` is sent as it is. +fake_repo topcoder1/sent "$(rules_json test)" +cp "$T/wf/ci.yml" "$T/gh/topcoder1/sent/workflows/ci#2.yml" +echo "contents/.github/workflows/ci%232.yml 404" > "$T/gh/topcoder1/sent/fail" +run_validator topcoder1/sent +if [[ $VRC -eq 3 ]] && + has "$T/v.err" "API error: GET /repos/topcoder1/sent/contents/.github/workflows/ci%232.yml failed (HTTP 404)" && + api_error_only; then + pass "i: a failed read of 'ci#2.yml' names its endpoint as sent" +else + fail "i: the 404 on 'ci#2.yml' should name .../ci%232.yml (got $VRC)" + show "$T/v.out" "$T/v.err" +fi +fake_repo topcoder1/sent2 "$(rules_json test)" +cp "$T/wf/ci.yml" "$T/gh/topcoder1/sent2/workflows/ci#2.yml" +answer topcoder1/sent2 "ci#2.yml" '{"type":"file","name":"ci#2.yml","encoding":"base64","content":"PRIVATE-BODY-CANARY not base64!"}' +run_validator topcoder1/sent2 +if [[ $VRC -eq 3 ]] && + has "$T/v.err" "API error: GET /repos/topcoder1/sent2/contents/.github/workflows/ci%232.yml did not answer base64 file content" && + api_error_only && no_body_bytes; then + pass "i: a 'ci#2.yml' answer that is not base64 content names its endpoint as sent" +else + fail "i: the bad answer for 'ci#2.yml' should name .../ci%232.yml (got $VRC)" + show "$T/v.out" "$T/v.err" +fi +fake_repo topcoder1/sent3 '[]' ci.yml +mkdir -p "$T/gh/topcoder1/sent3/branch-rules/rel/main#x" +echo '{"message":"not a list","leak":"PRIVATE-BODY-CANARY-branch"}' > "$T/gh/topcoder1/sent3/branch-rules/rel/main#x/rules.json" +run_validator topcoder1/sent3 --branch "rel/main#x" +if [[ $VRC -eq 3 ]] && + has "$T/v.err" "API error: GET /repos/topcoder1/sent3/rules/branches/rel/main%23x did not answer an array of rules" && + api_error_only && no_body_bytes; then + pass "i: a rules answer for 'rel/main#x' that is not a list names its endpoint as sent" +else + fail "i: the bad rules answer for 'rel/main#x' should name .../rel/main%23x (got $VRC)" + show "$T/v.out" "$T/v.err" +fi + # --------------------------------------------------------------------------- # Negative controls: the old code, verbatim from origin/main at bd86424 # (scripts/validate-required-checks.sh lines 61-72, 94-95, 97-98), in its own @@ -770,6 +1076,105 @@ else fail "n3: the old per-file fetch should swallow the 404 (got rc $NRC)" fi +# --------------------------------------------------------------------------- +# Shim controls. Cases h and i tell the fix from the bug only while the shim +# reads a request the way gh and GitHub do (measured: see the shim). These pin +# it on the requests origin/main at 71322c5 sent, the rules call without +# --paginate and each value as it was given, and on both forms --paginate +# prints. +# --------------------------------------------------------------------------- +shim() { # endpoint [gh args...] -> $SHRC, $T/s.out, $T/s.err + SHRC=0 + GH_FAKE="$T/gh" "$T/bin/gh" api "$@" > "$T/s.out" 2> "$T/s.err" < /dev/null || SHRC=$? +} + +fake_repo topcoder1/rules31 "$(rules_after 30 test)" +shim /repos/topcoder1/rules31/rules/branches/main +s1_first=$(jq -c '[length, any(.[]; .type == "required_status_checks")]' "$T/s.out" 2> /dev/null || true) +shim "/repos/topcoder1/rules31/rules/branches/main?page=2" +s1_second=$(jq -c 'map(.ruleset_id)' "$T/s.out" 2> /dev/null || true) +shim "/repos/topcoder1/rules31/rules/branches/main?per_page=100" +s1_whole=$(jq -c length "$T/s.out" 2> /dev/null || true) +shim "/repos/topcoder1/rules31/rules/branches/main?per_page=1&page=2" +s1_amp=$(jq -c 'map(.ruleset_id)' "$T/s.out" 2> /dev/null || true) +shim "/repos/topcoder1/rules31/rules/branches/main?per_page=1%zz&page=2" +s1_bad=$(jq -c 'map(.ruleset_id)' "$T/s.out" 2> /dev/null || true) +if [[ $s1_first == "[30,false]" && $s1_second == "[31]" && $s1_whole == 31 && + $s1_amp == "[2]" && $s1_bad == "[31]" ]]; then + pass "s1: the rules answer comes 30 rules a page; per_page and page select, & splits, a bad % drops its parameter" +else + fail "s1: expected [30,false] [31] 31 [2] [31], got $s1_first $s1_second $s1_whole $s1_amp $s1_bad" +fi + +fake_repo topcoder1/rules101 "$(rules_after 100 test)" +shim /repos/topcoder1/rules101/rules/branches/main --paginate +s2_one=$(jq -c length "$T/s.out" 2> /dev/null | tr '\n' ' ' || true) +touch "$T/gh/topcoder1/rules101/pages_apart" +shim /repos/topcoder1/rules101/rules/branches/main --paginate +s2_apart=$(jq -c length "$T/s.out" 2> /dev/null | tr '\n' ' ' || true) +if [[ $s2_one == "101 " && $s2_apart == "100 1 " ]]; then + pass "s2: --paginate asks for 100 rules a page and prints every page, as one array or an array per page" +else + fail "s2: expected page lengths '101 ' and '100 1 ', got '$s2_one' and '$s2_apart'" +fi + +fake_repo topcoder1/pagebytes '[]' +pb="$T/gh/topcoder1/pagebytes/rule-pages" +mkdir -p "$pb" +printf '%s' '[{"type":"deletion"}]' > "$pb/1.json" +printf '%s' null > "$pb/2.json" +shim /repos/topcoder1/pagebytes/rules/branches/main +s3_plain="$SHRC $(cat "$T/s.out")" +shim /repos/topcoder1/pagebytes/rules/branches/main --paginate +s3_apart="$SHRC $(cat "$T/s.out")" +printf '%s' '[{"type":"deletion"}' > "$pb/1.json" +shim /repos/topcoder1/pagebytes/rules/branches/main --paginate +s3_open="$SHRC $(cat "$T/s.out")" +rm "$pb/2.json" +echo 403 > "$pb/2.fail" +shim /repos/topcoder1/pagebytes/rules/branches/main --paginate +s3_fail="$SHRC $(cat "$T/s.out")" +if [[ $s3_plain == '0 [{"type":"deletion"}]' && $s3_apart == '0 [{"type":"deletion"}]null' && + $s3_open == '0 [{"type":"deletion"}null' && + $s3_fail == '1 [{"type":"deletion"}{"message":"Resource not accessible by personal access token",'* ]] && + has "$T/s.err" "gh: Resource not accessible by personal access token (HTTP 403)"; then + pass "s3: explicit pages print as given: one page, an array per page, gh 2.97.0's open array, a failing page 2" +else + fail "s3: unexpected page bytes" + printf '%s\n' "$s3_plain" "$s3_apart" "$s3_open" "$s3_fail" +fi + +fake_repo topcoder1/unencoded '[]' ci.yml +uw="$T/gh/topcoder1/unencoded/workflows" +printf 'cut\n' > "$uw/ci" +for name in "ci#2.yml" "ci?2.yml" "50%.yml"; do cp "$T/wf/ci.yml" "$uw/$name"; done +for branch in release relAse; do + mkdir -p "$T/gh/topcoder1/unencoded/branch-rules/$branch" + rules_json "from $branch" > "$T/gh/topcoder1/unencoded/branch-rules/$branch/rules.json" +done +unencoded() { # endpoint after /repos/topcoder1/unencoded/, a jq filter, what it must read + shim "/repos/topcoder1/unencoded/$1" + local got + got=$(jq -r "$2" "$T/s.out" 2> /dev/null || true) + if [[ $SHRC -eq 0 && $got == "$3" ]]; then + pass "s4: an unencoded '$1' reads $3" + else + fail "s4: '$1' should read $3, got '$got' (rc $SHRC)" + show "$T/s.err" + fi +} +unencoded "contents/.github/workflows/ci#2.yml" .name ci +unencoded "contents/.github/workflows/ci?2.yml" .name ci +unencoded "rules/branches/release#2" '.[1].parameters.required_status_checks[0].context' "from release" +unencoded "rules/branches/rel%41se" '.[1].parameters.required_status_checks[0].context' "from relAse" +shim "/repos/topcoder1/unencoded/contents/.github/workflows/50%.yml" +if [[ $SHRC -eq 1 && ! -s "$T/s.out" ]] && has "$T/s.err" "invalid URL escape"; then + pass "s4: an unencoded '50%.yml' fails in gh, an invalid URL escape, before anything is sent" +else + fail "s4: '50%.yml' should fail in gh with rc 1 and no answer (got rc $SHRC)" + show "$T/s.out" "$T/s.err" +fi + echo if [[ $failed -ne 0 ]]; then echo "FAILED" From 0a15680b30f1d4ab2c6131f01e60e65d1e84bafd Mon Sep 17 00:00:00 2001 From: topcoder1 Date: Sat, 17 Oct 2026 12:02:04 -0700 Subject: [PATCH 2/7] fix(audit-required-checks): read every rules page; percent-encode --branch and file names The weekly fleet audit runs this validator across topcoder1 and whois-api-llc, so each gap was a silent pass that hid a phantom required check fleet-wide. - The rules call ran without --paginate, and the endpoint pages its answer 30 rules a page: a required_status_checks rule past page 1 read as no required checks, exit 0. It now pages, every printed page must be an array (`add` would take a null page for nothing), and the shape check runs on the merged list (`jq -e` goes by the last of several values). - --branch and each workflow file name went into the URL as they were: gh never sends a #fragment, `?` starts the query, and a bad `%` fails inside gh. A new uri() percent-encodes them, `/` kept, so ordinary values send the same bytes as before. Error messages name the endpoint as sent. Exit codes and the no-response-bytes rule from #299 are unchanged. Mirrors topcoder1/dotclaude#525 for this copy, which has no --ref. Co-Authored-By: Claude Opus 5.5 --- scripts/validate-required-checks.sh | 41 ++++++++++++++++++++++------- selftest/README.md | 13 ++++++++- 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/scripts/validate-required-checks.sh b/scripts/validate-required-checks.sh index 784329f..bb461b8 100755 --- a/scripts/validate-required-checks.sh +++ b/scripts/validate-required-checks.sh @@ -140,15 +140,37 @@ api_get() { # [--allow-404] [extra gh api args...] exit 3 } +# uri : percent-encoded, all but `/`, to go into the endpoint +# of a gh api call. gh parses the endpoint as a URL, so a value spliced in as +# it is can read something else: gh never sends what follows a `#`, a `?` +# starts the query, and a `%` that starts no %XX escape fails inside gh +# before anything is sent, while GitHub decodes any %XX that is left. +# `--branch release#2` read release's rules, and a workflow file named +# `ci#2.yml` was read as `ci`. A `/` is sent as it is, so a branch with one +# reads exactly as it always did. Call it in an assignment: inside a +# command's arguments its failure would not stop the command. +uri() { + jq -rn --arg v "$1" '$v | @uri | split("%2F") | join("/")' +} + # 1. Fetch ruleset for the branch and pull required contexts. # Use the branch-rules endpoint (lists rulesets that apply to a branch), -# then filter to the matching ruleset name. -api_get "/repos/$REPO/rules/branches/$BRANCH" -RULESET_DATA="$API_BODY" +# then filter to the matching ruleset name. It pages its answer, 30 rules +# a page, and the required_status_checks rule can be on any page: read +# without --paginate, one past page 1 was never seen, and the branch passed +# as having no required checks. gh 2.97.0 prints every page as one array; +# its help, and older gh, describe an array per page. `jq -s add` merges +# either into one list, once every page is an array: `add` takes a null +# page for nothing at all, so a null page would merge away unseen. +RULES_ENDPOINT="/repos/$REPO/rules/branches/$(uri "$BRANCH")" +api_get "$RULES_ENDPOINT" --paginate # An array of rule objects, or exit 3: the filter below must never run on -# anything else. `[]` (no rule applies) passes and exits 0 further down. -if ! jq -e 'type == "array" and all(.[]; type == "object")' >/dev/null 2>&1 <<< "$RULESET_DATA"; then - echo " API error: GET /repos/$REPO/rules/branches/$BRANCH did not answer an array of rules" >&2 +# anything else. The check runs on the merged pages, because `jq -e` given +# several values goes by the last one only. `[]` (no rule applies) passes and +# exits 0 further down. +if ! RULESET_DATA=$(jq -cs 'if all(.[]; type == "array") then add else error("a page is not a list") end' 2>/dev/null <<< "$API_BODY") || + ! jq -e 'type == "array" and all(.[]; type == "object")' >/dev/null 2>&1 <<< "$RULESET_DATA"; then + echo " API error: GET $RULES_ENDPOINT did not answer an array of rules" >&2 exit 3 fi CONTEXTS=$(echo "$RULESET_DATA" | jq -r \ @@ -177,7 +199,7 @@ dump_workflows() { local repo="$1" local out_dir="$2" mkdir -p "$out_dir" - local names f content + local names f content endpoint if ! api_get --allow-404 "/repos/$repo/contents/.github/workflows"; then # 404: no workflows directory, which is a state to audit (its required # checks have no producer here). But a private repo also answers 404 to a @@ -199,7 +221,8 @@ dump_workflows() { # the script. while IFS= read -r f; do [[ -z "$f" ]] && continue - api_get "/repos/$repo/contents/.github/workflows/$f" + endpoint="/repos/$repo/contents/.github/workflows/$(uri "$f")" + api_get "$endpoint" # Base64 file content, or exit 3. A directory under that name, a file # over 1 MB (`encoding: none`, no inline content) and malformed base64 # would otherwise leave an empty or garbled workflow, which reads as a @@ -208,7 +231,7 @@ dump_workflows() { # the end, once GitHub's line breaks are dropped. if ! content=$(jq -er 'select(type == "object" and .encoding == "base64" and (.content | type) == "string" and (.content | gsub("\\s"; "") | test("^(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?$"))) | .content' 2>/dev/null <<< "$API_BODY") || ! printf '%s\n' "$content" | base64 -d > "$out_dir/$f" 2>/dev/null; then - echo " API error: GET /repos/$repo/contents/.github/workflows/$f did not answer base64 file content" >&2 + echo " API error: GET $endpoint did not answer base64 file content" >&2 exit 3 fi done <<< "$names" diff --git a/selftest/README.md b/selftest/README.md index 9023561..62fad44 100644 --- a/selftest/README.md +++ b/selftest/README.md @@ -273,7 +273,18 @@ arbitrary helper scripts; that's a different kind of repo. may use `--jq` (gh's own filter error quotes the response; the shim reproduces that). Negative controls run the old fetches verbatim against the same answers: jq exit 5, a silent "no branch rules found" exit 0, an - empty listing, an empty workflow file. + empty listing, an empty workflow file. Two more gaps, closed 2026-10-17, + each a silent pass. The rules call read one page of the branch's rules, + 30 a page, so a `required_status_checks` rule past page 1 read as no + required checks, exit 0: a rule at 31 or at 101 is now validated, in both + forms `gh api --paginate` prints (one array, gh 2.97.0; an array per page, + gh's help), and a bad rule on page 1, a `null` page 2 and a page 2 that + fails all exit 3. And `--branch` and workflow file names went into the URL + unencoded, so `#` (a fragment gh never sends), `?` and a bad `%` read + another branch or file, or failed in gh: they are now percent-encoded, + `/` kept, and an API error names the endpoint as sent. The shim reads an + endpoint the way gh and GitHub were measured to, and shim controls pin + that on the unpaginated, unencoded requests. - `test_pr_files_listing.sh` — no reusable may fetch changed files via `gh pr diff` (HTTP 406 past 20k diff lines); pins the paginated files-API idiom instead. From db6882584008b925427acc9e7f7afca91634d745 Mon Sep 17 00:00:00 2001 From: topcoder1 Date: Sat, 17 Oct 2026 12:22:50 -0700 Subject: [PATCH 3/7] test(audit-required-checks): no page-1 bytes on a failing page 2; gh's {repo} placeholder From Codex pre-review round 4: - The failing-page-2 case now carries a canary in page 1 and checks that none of it reaches the output: gh --paginate prints the pages before the failure on stdout, and only this case would catch an api_get that echoed them (mutation checked). - gh fills {owner}, {repo} and {branch} in an endpoint from the checkout it runs in (measured 2026-10-17, gh 2.97.0): unencoded, a workflow named `{repo}.yml` was read as `ci-workflows.yml`. The shim fills them as for the scheduled audit, and a `{repo}.yml` case pins that braces are encoded. Co-Authored-By: Claude Opus 5.5 --- selftest/test_audit_required_checks.sh | 43 +++++++++++++++++--------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/selftest/test_audit_required_checks.sh b/selftest/test_audit_required_checks.sh index 2dc637e..3959f8b 100644 --- a/selftest/test_audit_required_checks.sh +++ b/selftest/test_audit_required_checks.sh @@ -56,9 +56,10 @@ # a bad rule on page 1 of 2, a page 2 that is not a list, and a page 2 # that fails each exit 3, echoing none of the answer # i --branch and workflow file names go into the URL percent-encoded, `/` -# kept (2026-10-17): a file name with `#`, `?`, `%` or a space is read as -# that file, a branch with `#` or `%41` is read as that branch, and an -# API error names the endpoint as it was sent +# kept (2026-10-17): a file name with `#`, `?`, `%`, gh's `{repo}` +# placeholder or a space is read as that file, a branch with `#` or +# `%41` is read as that branch, and an API error names the endpoint as +# it was sent # Negative controls (the shim reproduces each old failure, run verbatim): # n1 the old rules fetch + filter dies with jq exit 5 on case a's 401 # n4 the old rules fetch reads case a's no-response failure as "no branch @@ -72,8 +73,8 @@ # s1 the rules answer comes 30 rules a page, and per_page and page select # s2 --paginate prints every page, as one array or as an array per page # s3 explicit page bytes print as given, a failing page included -# s4 an unencoded `#`, `?` or bad `%` reads another file or fails in gh, -# and an unencoded branch reads another branch's rules +# s4 an unencoded `#`, `?`, bad `%` or `{repo}` reads another file or +# fails in gh, and an unencoded branch reads another branch's rules # # Run from the repo root: # bash selftest/test_audit_required_checks.sh @@ -113,7 +114,12 @@ show() { cat -- "$@" 2>/dev/null || true; } # # It reads an endpoint the way gh 2.97.0 and GitHub did, measured 2026-10-16 # on topcoder1/dotclaude and against a local server, and 2026-10-17 on this -# repo. gh parses the endpoint as a URL. It never sends a #fragment +# repo. gh first fills the {owner}, {repo} and {branch} placeholders from the +# checkout it runs in (in this repo's worktree, `contents/{repo}.yml` +# requested `contents/ci-workflows.yml`; outside any checkout gh fails, +# `unable to expand placeholder in path`); the shim fills them as for the +# scheduled audit, this repo's checkout on main. `%7Brepo%7D` is sent as it +# is. Then gh parses the endpoint as a URL. It never sends a #fragment # (`rules/branches/main#x` read main's rules), and a % in the path that starts # no %XX escape fails in gh before anything is sent (`invalid URL escape # "%zz"`, rc 1). GitHub decodes %XX in the path (`rules/branches/ma%69n` read @@ -135,7 +141,10 @@ if [ "${1:-}" != api ]; then echo "shim: unexpected gh $*" >&2 exit 98 fi -url="${2%%#*}" +url="${2//\{owner\}/topcoder1}" +url="${url//\{repo\}/ci-workflows}" +url="${url//\{branch\}/main}" +url="${url%%#*}" bad_escape='%($|[^0-9A-Fa-f]|[0-9A-Fa-f]($|[^0-9A-Fa-f]))' if grep -Eq "$bad_escape" <<< "${url%%\?*}"; then echo "parse \"https://api.github.com/${url#/}\": invalid URL escape" >&2 @@ -901,15 +910,16 @@ for form in array-per-page gh-2.97.0; do fi # A page 2 that fails: gh --paginate exits 1 there, after printing the - # pages before it, and page 1's rules must never be judged on their own. + # pages before it, and page 1's rules must never be judged on their own, + # nor printed. r="topcoder1/failpage-$form" fake_repo "$r" '[]' ci.yml - rule_pages "$r" "$form" "$(rules_after 29 test)" 403 + rule_pages "$r" "$form" "$(rules_after 29 test | sed 's#"o/r"#"PRIVATE-BODY-CANARY-page1"#g')" 403 run_validator "$r" if [[ $VRC -eq 3 ]] && has "$T/v.err" "API error: GET /repos/$r/rules/branches/main failed (HTTP 403)" && has "$T/v.err" "gh: Resource not accessible by personal access token (HTTP 403)" && - api_error_only; then + api_error_only && no_body_bytes; then pass "h: a page 2 that fails exits 3 naming its status ($form)" else fail "h: a page 2 that fails ($form) should exit 3 with HTTP 403 (got $VRC)" @@ -925,10 +935,11 @@ done # %XX that is left. A `/` is sent as it is. # --------------------------------------------------------------------------- # Each file is its repo's only producer of the required check. Unencoded, -# `#` and `?` cut the name to `ci`, a file the repo does not have, and `%.` -# stopped gh. A space always worked: gh escapes it itself. +# `#` and `?` cut the name to `ci`, a file the repo does not have, `%.` +# stopped gh, and gh filled `{repo}` in with the checkout's repo name. A space +# always worked: gh escapes it itself. n=0 -for name in "ci#2.yml" "ci?2.yml" "50%.yml" "ci 2.yml"; do +for name in "ci#2.yml" "ci?2.yml" "50%.yml" "{repo}.yml" "ci 2.yml"; do n=$((n + 1)) r="topcoder1/filename$n" fake_repo "$r" "$(rules_json test)" @@ -1147,8 +1158,8 @@ fi fake_repo topcoder1/unencoded '[]' ci.yml uw="$T/gh/topcoder1/unencoded/workflows" printf 'cut\n' > "$uw/ci" -for name in "ci#2.yml" "ci?2.yml" "50%.yml"; do cp "$T/wf/ci.yml" "$uw/$name"; done -for branch in release relAse; do +for name in "ci#2.yml" "ci?2.yml" "50%.yml" "ci-workflows.yml"; do cp "$T/wf/ci.yml" "$uw/$name"; done +for branch in release relAse main; do mkdir -p "$T/gh/topcoder1/unencoded/branch-rules/$branch" rules_json "from $branch" > "$T/gh/topcoder1/unencoded/branch-rules/$branch/rules.json" done @@ -1167,6 +1178,8 @@ unencoded "contents/.github/workflows/ci#2.yml" .name ci unencoded "contents/.github/workflows/ci?2.yml" .name ci unencoded "rules/branches/release#2" '.[1].parameters.required_status_checks[0].context' "from release" unencoded "rules/branches/rel%41se" '.[1].parameters.required_status_checks[0].context' "from relAse" +unencoded "contents/.github/workflows/{repo}.yml" .name ci-workflows.yml +unencoded "rules/branches/{branch}" '.[1].parameters.required_status_checks[0].context' "from main" shim "/repos/topcoder1/unencoded/contents/.github/workflows/50%.yml" if [[ $SHRC -eq 1 && ! -s "$T/s.out" ]] && has "$T/s.err" "invalid URL escape"; then pass "s4: an unencoded '50%.yml' fails in gh, an invalid URL escape, before anything is sent" From d7ee6e51b70cf41f2b098a28159e429cbfec42a6 Mon Sep 17 00:00:00 2001 From: topcoder1 Date: Sat, 17 Oct 2026 12:30:20 -0700 Subject: [PATCH 4/7] test(audit-required-checks): gh's :repo placeholder too From Codex pre-review round 5: gh 2.97.0 also fills :owner, :repo and :branch where no letter, digit or `_` follows (its placeholderRE, measured 2026-10-17: `contents/:repo.yml` requested `contents/ci-workflows.yml`, `:repository.yml` was sent as it was). The shim fills both forms, and a `:repo.yml` case pins that `:` is encoded (an encoder that left it would pass every other case; mutation checked). Co-Authored-By: Claude Opus 5.5 --- selftest/test_audit_required_checks.sh | 52 ++++++++++++++------------ 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/selftest/test_audit_required_checks.sh b/selftest/test_audit_required_checks.sh index 3959f8b..9253655 100644 --- a/selftest/test_audit_required_checks.sh +++ b/selftest/test_audit_required_checks.sh @@ -56,8 +56,8 @@ # a bad rule on page 1 of 2, a page 2 that is not a list, and a page 2 # that fails each exit 3, echoing none of the answer # i --branch and workflow file names go into the URL percent-encoded, `/` -# kept (2026-10-17): a file name with `#`, `?`, `%`, gh's `{repo}` -# placeholder or a space is read as that file, a branch with `#` or +# kept (2026-10-17): a file name with `#`, `?`, `%`, gh's `{repo}` or +# `:repo` placeholder or a space is read as that file, a branch with `#` or # `%41` is read as that branch, and an API error names the endpoint as # it was sent # Negative controls (the shim reproduces each old failure, run verbatim): @@ -73,8 +73,8 @@ # s1 the rules answer comes 30 rules a page, and per_page and page select # s2 --paginate prints every page, as one array or as an array per page # s3 explicit page bytes print as given, a failing page included -# s4 an unencoded `#`, `?`, bad `%` or `{repo}` reads another file or -# fails in gh, and an unencoded branch reads another branch's rules +# s4 an unencoded `#`, `?`, bad `%`, `{repo}` or `:repo` reads another file +# or fails in gh, and an unencoded branch reads another branch's rules # # Run from the repo root: # bash selftest/test_audit_required_checks.sh @@ -114,23 +114,25 @@ show() { cat -- "$@" 2>/dev/null || true; } # # It reads an endpoint the way gh 2.97.0 and GitHub did, measured 2026-10-16 # on topcoder1/dotclaude and against a local server, and 2026-10-17 on this -# repo. gh first fills the {owner}, {repo} and {branch} placeholders from the -# checkout it runs in (in this repo's worktree, `contents/{repo}.yml` -# requested `contents/ci-workflows.yml`; outside any checkout gh fails, -# `unable to expand placeholder in path`); the shim fills them as for the -# scheduled audit, this repo's checkout on main. `%7Brepo%7D` is sent as it -# is. Then gh parses the endpoint as a URL. It never sends a #fragment -# (`rules/branches/main#x` read main's rules), and a % in the path that starts -# no %XX escape fails in gh before anything is sent (`invalid URL escape -# "%zz"`, rc 1). GitHub decodes %XX in the path (`rules/branches/ma%69n` read -# main's rules) and in the query, where `&` starts the next parameter and a -# parameter with a bad % is dropped. It pages the rules answer, 30 rules a -# page unless per_page asks for up to 100. gh --paginate asks for 100 unless -# the endpoint names a per_page, follows every next page, and prints the pages -# of an array as one array, the `]` only after the last page (a null page 2 -# printed `[{"type":"deletion"}null`, rc 0; a 403 on page 2 printed page 1, -# then the error body, rc 1). gh's help instead describes an array per page, -# the form `pages_apart` gives. +# repo. gh first fills the placeholders {owner}, {repo} and {branch}, and +# :owner, :repo and :branch where no letter, digit or `_` follows, from the +# checkout it runs in: in this repo's worktree `contents/{repo}.yml` and +# `contents/:repo.yml` requested `contents/ci-workflows.yml`, while +# `:repository.yml`, `%7Brepo%7D.yml` and `%3Arepo.yml` were sent as they +# were, and outside any checkout gh fails (`unable to expand placeholder in +# path`). The shim fills them as for the scheduled audit, this repo's +# checkout on main. Then gh parses the endpoint as a URL. It never sends a +# #fragment (`rules/branches/main#x` read main's rules), and a % in the path +# that starts no %XX escape fails in gh before anything is sent (`invalid URL +# escape "%zz"`, rc 1). GitHub decodes %XX in the path +# (`rules/branches/ma%69n` read main's rules) and in the query, where `&` +# starts the next parameter and a parameter with a bad % is dropped. It pages +# the rules answer, 30 rules a page unless per_page asks for up to 100. gh +# --paginate asks for 100 unless the endpoint names a per_page, follows every +# next page, and prints the pages of an array as one array, the `]` only +# after the last page (a null page 2 printed `[{"type":"deletion"}null`, rc 0; +# a 403 on page 2 printed page 1, then the error body, rc 1). gh's help +# instead describes an array per page, the form `pages_apart` gives. # --------------------------------------------------------------------------- mkdir -p "$T/bin" "$T/gh" cat > "$T/bin/gh" <<'STUB' @@ -144,6 +146,7 @@ fi url="${2//\{owner\}/topcoder1}" url="${url//\{repo\}/ci-workflows}" url="${url//\{branch\}/main}" +url=$(printf '%s' "$url" | sed -E 's/:owner([^0-9A-Za-z_]|$)/topcoder1\1/g; s/:repo([^0-9A-Za-z_]|$)/ci-workflows\1/g; s/:branch([^0-9A-Za-z_]|$)/main\1/g') url="${url%%#*}" bad_escape='%($|[^0-9A-Fa-f]|[0-9A-Fa-f]($|[^0-9A-Fa-f]))' if grep -Eq "$bad_escape" <<< "${url%%\?*}"; then @@ -936,10 +939,10 @@ done # --------------------------------------------------------------------------- # Each file is its repo's only producer of the required check. Unencoded, # `#` and `?` cut the name to `ci`, a file the repo does not have, `%.` -# stopped gh, and gh filled `{repo}` in with the checkout's repo name. A space -# always worked: gh escapes it itself. +# stopped gh, and gh filled `{repo}` and `:repo` in with the checkout's repo +# name. A space always worked: gh escapes it itself. n=0 -for name in "ci#2.yml" "ci?2.yml" "50%.yml" "{repo}.yml" "ci 2.yml"; do +for name in "ci#2.yml" "ci?2.yml" "50%.yml" "{repo}.yml" ":repo.yml" "ci 2.yml"; do n=$((n + 1)) r="topcoder1/filename$n" fake_repo "$r" "$(rules_json test)" @@ -1179,6 +1182,7 @@ unencoded "contents/.github/workflows/ci?2.yml" .name ci unencoded "rules/branches/release#2" '.[1].parameters.required_status_checks[0].context' "from release" unencoded "rules/branches/rel%41se" '.[1].parameters.required_status_checks[0].context' "from relAse" unencoded "contents/.github/workflows/{repo}.yml" .name ci-workflows.yml +unencoded "contents/.github/workflows/:repo.yml" .name ci-workflows.yml unencoded "rules/branches/{branch}" '.[1].parameters.required_status_checks[0].context' "from main" shim "/repos/topcoder1/unencoded/contents/.github/workflows/50%.yml" if [[ $SHRC -eq 1 && ! -s "$T/s.out" ]] && has "$T/s.err" "invalid URL escape"; then From f0fad3bcbd91f5558bc1f8d352b1bc7657243d49 Mon Sep 17 00:00:00 2001 From: topcoder1 Date: Sat, 17 Oct 2026 12:33:36 -0700 Subject: [PATCH 5/7] docs(selftest): name gh's placeholders among the unencoded values Co-Authored-By: Claude Opus 5.5 --- selftest/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/selftest/README.md b/selftest/README.md index 62fad44..f7f18b4 100644 --- a/selftest/README.md +++ b/selftest/README.md @@ -280,7 +280,8 @@ arbitrary helper scripts; that's a different kind of repo. forms `gh api --paginate` prints (one array, gh 2.97.0; an array per page, gh's help), and a bad rule on page 1, a `null` page 2 and a page 2 that fails all exit 3. And `--branch` and workflow file names went into the URL - unencoded, so `#` (a fragment gh never sends), `?` and a bad `%` read + unencoded, so `#` (a fragment gh never sends), `?`, a bad `%` and gh's + `{repo}` / `:repo` placeholders (filled from the checkout gh runs in) read another branch or file, or failed in gh: they are now percent-encoded, `/` kept, and an API error names the endpoint as sent. The shim reads an endpoint the way gh and GitHub were measured to, and shim controls pin From 49d23c26dc33569a8b2f60cfdfe40105ed55c699 Mon Sep 17 00:00:00 2001 From: topcoder1 Date: Sat, 17 Oct 2026 12:41:00 -0700 Subject: [PATCH 6/7] test(audit-required-checks): shim pages and query errors as gh and GitHub give them From the independent review, both measured 2026-10-17: - A bad % in the rules endpoint's query gets HTTP 400 (`gh: HTTP 400`, no stdout, rc 1); only the contents endpoint drops that parameter. Control s1 had asserted the drop for rules. - rule-pages/ now holds GitHub's own pages, and the shim prints them as gh does: without --paginate the one page as GitHub sent it, and with it joined as gh 2.97.0's paginatedArrayReader joins them (a page with another after it loses its `]`, a later page's `[` becomes `,`), or an array per page. The fixtures had held gh's --paginate bytes, so the old, unpaginated validator saw an unclosed page 1 and looked fail-closed: run as it really was, it exits 1 on the null page and 0 on the failing page 2, and both cases now fail on it (18 of 70, both bashes). Co-Authored-By: Claude Opus 5.5 --- selftest/test_audit_required_checks.sh | 113 ++++++++++++++++--------- 1 file changed, 71 insertions(+), 42 deletions(-) diff --git a/selftest/test_audit_required_checks.sh b/selftest/test_audit_required_checks.sh index 9253655..ea1f190 100644 --- a/selftest/test_audit_required_checks.sh +++ b/selftest/test_audit_required_checks.sh @@ -70,9 +70,11 @@ # reads a request as gh and GitHub do, so these pin it on the requests the # validator sent before them (the rules call without --paginate, each value # as it was given) and on both forms --paginate prints: -# s1 the rules answer comes 30 rules a page, and per_page and page select +# s1 the rules answer comes 30 rules a page, per_page and page select, and +# a bad % in its query gets HTTP 400 # s2 --paginate prints every page, as one array or as an array per page -# s3 explicit page bytes print as given, a failing page included +# s3 GitHub's pages print as gh prints them, with and without --paginate, +# a null and a failing page 2 included # s4 an unencoded `#`, `?`, bad `%`, `{repo}` or `:repo` reads another file # or fails in gh, and an unencoded branch reads another branch's rules # @@ -106,9 +108,9 @@ show() { cat -- "$@" 2>/dev/null || true; } # contents answer. With a `branch-rules/` directory, each branch's rules are # in branch-rules//rules.json and any other branch has none; # `pages_apart` prints a --paginate rules answer as an array per page; and -# `rule-pages/.json` holds the bytes gh prints for page of the rules -# answer, or `.fail` the status that page fails with. Every call is -# logged. Like gh, it applies a --jq filter itself, and a filter that fails +# `rule-pages/.json` holds GitHub's body for page of the rules answer, +# which the shim prints as gh would, or `.fail` the status that page fails +# with. Every call is logged. Like gh, it applies a --jq filter itself, and a filter that fails # prints part of the response in its error (measured 2026-10-16, gh 2.97.0: # `expected an object but got: array ([{"_links":{"git":"https: ...])`). # @@ -126,13 +128,18 @@ show() { cat -- "$@" 2>/dev/null || true; } # that starts no %XX escape fails in gh before anything is sent (`invalid URL # escape "%zz"`, rc 1). GitHub decodes %XX in the path # (`rules/branches/ma%69n` read main's rules) and in the query, where `&` -# starts the next parameter and a parameter with a bad % is dropped. It pages -# the rules answer, 30 rules a page unless per_page asks for up to 100. gh -# --paginate asks for 100 unless the endpoint names a per_page, follows every -# next page, and prints the pages of an array as one array, the `]` only -# after the last page (a null page 2 printed `[{"type":"deletion"}null`, rc 0; -# a 403 on page 2 printed page 1, then the error body, rc 1). gh's help -# instead describes an array per page, the form `pages_apart` gives. +# starts the next parameter. A query parameter with a bad % is dropped on the +# contents endpoint (`?ref=no-such%zz` read the default branch), and gets +# HTTP 400 on the rules endpoint (`gh: HTTP 400`, no stdout, rc 1). GitHub +# pages the rules answer, 30 rules a page unless per_page asks for up to 100. +# gh --paginate asks for 100 unless the endpoint names a per_page, follows +# every next page, and prints the pages of an array as one array (its +# paginatedArrayReader): a page with another after it loses its closing `]`, +# a later page's opening `[` becomes `,`, and anything else is printed as it +# came (a null page 2 printed `[{"type":"deletion"}null`, rc 0; a 403 on page +# 2 printed page 1, then the error body, rc 1). gh's help instead describes +# an array per page, the form `pages_apart` gives. Without --paginate, gh +# prints the one page as GitHub sent it. # --------------------------------------------------------------------------- mkdir -p "$T/bin" "$T/gh" cat > "$T/bin/gh" <<'STUB' @@ -170,7 +177,7 @@ for a in "$@"; do done pct_decode() { printf '%b' "${1//%/\\x}"; } path=$(pct_decode "${rest%%\?*}") -per_page="" page=1 +per_page="" page=1 bad_query=no case "$rest" in *\?*) query="${rest#*\?}" ;; *) query="" ;; @@ -181,7 +188,10 @@ while [ -n "$query" ]; do *'&'*) query="${query#*&}" ;; *) query="" ;; esac - if grep -Eq "$bad_escape" <<< "$param"; then continue; fi + if grep -Eq "$bad_escape" <<< "$param"; then + bad_query=yes + continue + fi case "$param" in per_page=*) per_page=$(pct_decode "${param#*=}") ;; page=*) page=$(pct_decode "${param#*=}") ;; @@ -204,6 +214,8 @@ respond() { # the answer on stdin -> stdout, through --jq when one was given } fail_with() { # status -> gh's stdout and stderr for that failure, rc=1 case "$1" in + 400) + echo "gh: HTTP 400" >&2 ;; 401) printf '{\n "message": "Bad credentials",\n "documentation_url": "https://docs.github.com/rest",\n "status": "401"\n}\n' echo "gh: Bad credentials (HTTP 401)" >&2 ;; @@ -228,21 +240,33 @@ if [ -f "$dir/fail" ]; then fi [ -d "$dir" ] || fail_with 404 serve_rules() { # rules file -> the rules answer, paged - local n from total + local n from total body more if [ -d "$dir/rule-pages" ]; then - # Page after page, as given; with --paginate, up to the first page that - # fails, which ends the call with that page's error. + # GitHub's pages, printed as gh prints them; with --paginate, up to the + # first page that fails, which ends the call with that page's error. n="$page" while :; do if [ -f "$dir/rule-pages/$n.fail" ]; then fail_with "$(cat "$dir/rule-pages/$n.fail")" fi - cat "$dir/rule-pages/$n.json" - n=$((n + 1)) - if [ "$paginate" = no ]; then break; fi - if [ ! -f "$dir/rule-pages/$n.json" ] && [ ! -f "$dir/rule-pages/$n.fail" ]; then - break + body=$(cat "$dir/rule-pages/$n.json") + more=no + if [ "$paginate" = yes ] && + { [ -f "$dir/rule-pages/$((n + 1)).json" ] || [ -f "$dir/rule-pages/$((n + 1)).fail" ]; }; then + more=yes + fi + if [ "$paginate" = yes ] && [ ! -f "$dir/pages_apart" ]; then + if [ "$n" != "$page" ]; then + case "$body" in + '[]'*) body=" ${body#?}" ;; + '['*) body=",${body#?}" ;; + esac + fi + if [ "$more" = yes ]; then body="${body%]}"; fi fi + printf '%s' "$body" + if [ "$more" = no ]; then break; fi + n=$((n + 1)) done return fi @@ -269,6 +293,7 @@ serve_rules() { # rules file -> the rules answer, paged } case "$path" in rules/branches/*) + if [ "$bad_query" = yes ]; then fail_with 400; fi branch="${path#rules/branches/}" if [ ! -d "$dir/branch-rules" ]; then serve_rules "$dir/rules.json" @@ -878,16 +903,13 @@ else show "$T/v.out" "$T/v.err" fi -rule_pages() { # owner/repo, page-1 form, page 1's rules, page 2's bytes or HTTP status - # Page 1 as gh prints it: an array of its own, or (gh 2.97.0) the array - # left open for the pages after it. +rule_pages() { # owner/repo, form, page 1's body, page 2's body or HTTP status + # GitHub's two pages, which gh --paginate prints in `form`: as one array + # (gh-2.97.0) or as an array per page. local d="$T/gh/$1/rule-pages" mkdir -p "$d" - if [[ $2 == gh-2.97.0 ]]; then - printf '%s' "${3%]}" > "$d/1.json" - else - printf '%s' "$3" > "$d/1.json" - fi + if [[ $2 == array-per-page ]]; then touch "$T/gh/$1/pages_apart"; fi + printf '%s' "$3" > "$d/1.json" case "$4" in [0-9][0-9][0-9]) echo "$4" > "$d/2.fail" ;; *) printf '%s' "$4" > "$d/2.json" ;; @@ -1112,12 +1134,13 @@ s1_whole=$(jq -c length "$T/s.out" 2> /dev/null || true) shim "/repos/topcoder1/rules31/rules/branches/main?per_page=1&page=2" s1_amp=$(jq -c 'map(.ruleset_id)' "$T/s.out" 2> /dev/null || true) shim "/repos/topcoder1/rules31/rules/branches/main?per_page=1%zz&page=2" -s1_bad=$(jq -c 'map(.ruleset_id)' "$T/s.out" 2> /dev/null || true) +s1_bad="$SHRC $(wc -c < "$T/s.out" | tr -d ' ')" if [[ $s1_first == "[30,false]" && $s1_second == "[31]" && $s1_whole == 31 && - $s1_amp == "[2]" && $s1_bad == "[31]" ]]; then - pass "s1: the rules answer comes 30 rules a page; per_page and page select, & splits, a bad % drops its parameter" + $s1_amp == "[2]" && $s1_bad == "1 0" ]] && has "$T/s.err" "gh: HTTP 400"; then + pass "s1: the rules answer comes 30 rules a page; per_page and page select, & splits, a bad % in the query gets HTTP 400" else - fail "s1: expected [30,false] [31] 31 [2] [31], got $s1_first $s1_second $s1_whole $s1_amp $s1_bad" + fail "s1: expected [30,false] [31] 31 [2] '1 0', got $s1_first $s1_second $s1_whole $s1_amp '$s1_bad'" + show "$T/s.err" fi fake_repo topcoder1/rules101 "$(rules_after 100 test)" @@ -1136,26 +1159,32 @@ fake_repo topcoder1/pagebytes '[]' pb="$T/gh/topcoder1/pagebytes/rule-pages" mkdir -p "$pb" printf '%s' '[{"type":"deletion"}]' > "$pb/1.json" -printf '%s' null > "$pb/2.json" +printf '%s' '[{"type":"creation"}]' > "$pb/2.json" shim /repos/topcoder1/pagebytes/rules/branches/main -s3_plain="$SHRC $(cat "$T/s.out")" +s3_one="$SHRC $(cat "$T/s.out")" +shim /repos/topcoder1/pagebytes/rules/branches/main --paginate +s3_joined="$SHRC $(cat "$T/s.out")" +touch "$T/gh/topcoder1/pagebytes/pages_apart" shim /repos/topcoder1/pagebytes/rules/branches/main --paginate s3_apart="$SHRC $(cat "$T/s.out")" -printf '%s' '[{"type":"deletion"}' > "$pb/1.json" +rm "$T/gh/topcoder1/pagebytes/pages_apart" +printf '%s' null > "$pb/2.json" shim /repos/topcoder1/pagebytes/rules/branches/main --paginate -s3_open="$SHRC $(cat "$T/s.out")" +s3_null="$SHRC $(cat "$T/s.out")" rm "$pb/2.json" echo 403 > "$pb/2.fail" shim /repos/topcoder1/pagebytes/rules/branches/main --paginate s3_fail="$SHRC $(cat "$T/s.out")" -if [[ $s3_plain == '0 [{"type":"deletion"}]' && $s3_apart == '0 [{"type":"deletion"}]null' && - $s3_open == '0 [{"type":"deletion"}null' && +if [[ $s3_one == '0 [{"type":"deletion"}]' && + $s3_joined == '0 [{"type":"deletion"},{"type":"creation"}]' && + $s3_apart == '0 [{"type":"deletion"}][{"type":"creation"}]' && + $s3_null == '0 [{"type":"deletion"}null' && $s3_fail == '1 [{"type":"deletion"}{"message":"Resource not accessible by personal access token",'* ]] && has "$T/s.err" "gh: Resource not accessible by personal access token (HTTP 403)"; then - pass "s3: explicit pages print as given: one page, an array per page, gh 2.97.0's open array, a failing page 2" + pass "s3: GitHub's pages print as gh prints them: one page alone, joined into one array or an array per page, a null and a failing page 2 joined in as they came" else fail "s3: unexpected page bytes" - printf '%s\n' "$s3_plain" "$s3_apart" "$s3_open" "$s3_fail" + printf '%s\n' "$s3_one" "$s3_joined" "$s3_apart" "$s3_null" "$s3_fail" fi fake_repo topcoder1/unencoded '[]' ci.yml From a34967370a07b525cf562d4ad0c892fc6e7c3d8e Mon Sep 17 00:00:00 2001 From: topcoder1 Date: Sat, 17 Oct 2026 12:50:06 -0700 Subject: [PATCH 7/7] test(audit-required-checks): pin gh's join of an empty page The independent review's optional nit: an empty `[]` page joins as ` ]` (gh's paginatedArrayReader); no fixture served one, so a shim that joined it as `,` passed. One more s3 row. Co-Authored-By: Claude Opus 5.5 --- selftest/test_audit_required_checks.sh | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/selftest/test_audit_required_checks.sh b/selftest/test_audit_required_checks.sh index ea1f190..a1ab7b2 100644 --- a/selftest/test_audit_required_checks.sh +++ b/selftest/test_audit_required_checks.sh @@ -1168,6 +1168,9 @@ touch "$T/gh/topcoder1/pagebytes/pages_apart" shim /repos/topcoder1/pagebytes/rules/branches/main --paginate s3_apart="$SHRC $(cat "$T/s.out")" rm "$T/gh/topcoder1/pagebytes/pages_apart" +printf '%s' '[]' > "$pb/2.json" +shim /repos/topcoder1/pagebytes/rules/branches/main --paginate +s3_empty="$SHRC $(cat "$T/s.out")" printf '%s' null > "$pb/2.json" shim /repos/topcoder1/pagebytes/rules/branches/main --paginate s3_null="$SHRC $(cat "$T/s.out")" @@ -1178,13 +1181,14 @@ s3_fail="$SHRC $(cat "$T/s.out")" if [[ $s3_one == '0 [{"type":"deletion"}]' && $s3_joined == '0 [{"type":"deletion"},{"type":"creation"}]' && $s3_apart == '0 [{"type":"deletion"}][{"type":"creation"}]' && + $s3_empty == '0 [{"type":"deletion"} ]' && $s3_null == '0 [{"type":"deletion"}null' && $s3_fail == '1 [{"type":"deletion"}{"message":"Resource not accessible by personal access token",'* ]] && has "$T/s.err" "gh: Resource not accessible by personal access token (HTTP 403)"; then - pass "s3: GitHub's pages print as gh prints them: one page alone, joined into one array or an array per page, a null and a failing page 2 joined in as they came" + pass "s3: GitHub's pages print as gh prints them: one page alone, joined into one array or an array per page, an empty page 2 as ' ]', a null and a failing page 2 joined in as they came" else fail "s3: unexpected page bytes" - printf '%s\n' "$s3_one" "$s3_joined" "$s3_apart" "$s3_null" "$s3_fail" + printf '%s\n' "$s3_one" "$s3_joined" "$s3_apart" "$s3_empty" "$s3_null" "$s3_fail" fi fake_repo topcoder1/unencoded '[]' ci.yml