From 83da44ccb13ddad4999de61e8ee747301d22e1c5 Mon Sep 17 00:00:00 2001 From: topcoder1 Date: Sat, 17 Oct 2026 14:55:28 -0700 Subject: [PATCH 1/2] test(audit-required-checks): a rule the filter cannot read is an API error (RED) Case j: the rules answer's shape check asked only for an array of objects, and the contexts filter skips a rule with no string type and reads a required_status_checks rule's checks with `[]?`. So a rule with no type, a required_status_checks rule with no list of checks, or a check with an empty context read as "no required_status_checks rule", exit 0, and the fleet audit filed the repo as clean and opened no issue. A check with no context read as a phantom named `null`, a check that is not an object crashed the filter (exit 4), and a context that was an object was printed, the answer's own bytes, into the public digest. Each of 14 malformed answers must exit 3 with stdout and stderr byte-equal to the header and one API error line naming the endpoint. A fleet run must file them as ERRORs that open the issue; a full answer and an empty list of checks must still exit 0. Four existing assertions take dotclaude's wording, "did not answer a valid list of rules". Against origin/main's validator (67e3588): 21 of 87 checks fail, the same under /bin/bash 3.2.57 and bash 5.3.15. Co-Authored-By: Claude Opus 5.5 --- selftest/test_audit_required_checks.sh | 126 ++++++++++++++++++++++++- 1 file changed, 122 insertions(+), 4 deletions(-) diff --git a/selftest/test_audit_required_checks.sh b/selftest/test_audit_required_checks.sh index a1ab7b2..11f62d2 100644 --- a/selftest/test_audit_required_checks.sh +++ b/selftest/test_audit_required_checks.sh @@ -60,6 +60,16 @@ # `: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 +# j every rule has a string type, and every required_status_checks rule +# lists its checks as objects with a non-empty string context +# (2026-10-17): a rule with no type or a number for one, a +# required_status_checks rule with no list, a string or an empty object +# for one, or a string for its parameters, a check that is a string, a +# check with no, an empty, a numeric or an object context, a bad check +# before or after a good one, and a bad rule beside a good one each exit +# 3 with the endpoint and nothing else; through the fleet they are +# ERRORs, and the digest step opens the issue. A full answer, and a rule +# with an empty list, still exit 0 # 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 @@ -491,7 +501,7 @@ fi for r in oddrules oddrules2; do run_validator "topcoder1/$r" if [[ $VRC -eq 3 ]] && - has "$T/v.err" "API error: GET /repos/topcoder1/$r/rules/branches/main did not answer an array of rules" && + has "$T/v.err" "API error: GET /repos/topcoder1/$r/rules/branches/main did not answer a valid list of rules" && ! grep -q "jq: error" "$T/v.err" && api_error_only && no_body_bytes; then pass "a: a 200 that is not an array of rules ($r) exits 3 and echoes none of it" else @@ -895,7 +905,7 @@ fake_repo topcoder1/badpage "$(rules_after 100 test | jq -c '.[0] = "PRIVATE-BOD 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" && + has "$T/v.err" "API error: GET /repos/topcoder1/badpage/rules/branches/main did not answer a valid list 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 @@ -926,7 +936,7 @@ for form in array-per-page gh-2.97.0; do 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" && + has "$T/v.err" "API error: GET /repos/$r/rules/branches/main did not answer a valid list 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 @@ -1029,7 +1039,7 @@ 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" && + has "$T/v.err" "API error: GET /repos/topcoder1/sent3/rules/branches/rel/main%23x did not answer a valid list 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 @@ -1037,6 +1047,114 @@ else show "$T/v.out" "$T/v.err" fi +# --------------------------------------------------------------------------- +# j. A rule the filter cannot read is an API error, never "no required checks" +# (2026-10-17). The shape check asked for an array of objects and no more, +# and the filter keeps the rules whose type is required_status_checks and +# reads their checks with `[]?`: a rule with no string type, a +# required_status_checks rule with no list of checks, or a check with an +# empty context read as "no required_status_checks rule", exit 0, so the +# fleet filed the repo as clean and opened no issue, and whatever check +# the rule required was never validated. A check with no context, or a +# number for one, read as a phantom named `null` or `5`; a check that is +# not an object crashed the filter, exit 4; and a context that was an +# object was printed, the answer's own bytes, into the public digest. +# dotclaude's copy has required this shape since topcoder1/dotclaude#521. +# --------------------------------------------------------------------------- +# Each answer is clean-a's with one defect, every ruleset_source in it a +# canary, which an edit can also use as $c. A jq error inside fake_repo's +# arguments would serve an empty answer, which exits 3 with the same message: +# a case that tested nothing would pass. So a broken edit fails here. +malformed() { # owner/repo, a jq edit of clean-a's rules answer + local answer + if answer=$(rules_json test | + jq -c --arg c "PRIVATE-BODY-CANARY-${1#*/}" "map(.ruleset_source = \$c) | $2") && + jq -e --arg c "PRIVATE-BODY-CANARY-${1#*/}" 'type == "array" and (tostring | contains($c))' \ + > /dev/null 2>&1 <<< "$answer"; then + fake_repo "$1" "$answer" ci.yml + else + fail "j: the edit '$2' did not build a rules answer for $1" + fi +} +malformed topcoder1/notype 'del(.[1].type)' +malformed topcoder1/numtype '.[1].type = 7' +malformed topcoder1/nolist '.[1].parameters = {}' +malformed topcoder1/stringlist '.[1].parameters.required_status_checks = "test"' +# `[]?` iterates an object's values as it does a list's elements. +malformed topcoder1/objectlist '.[1].parameters.required_status_checks = {}' +malformed topcoder1/stringcheck '.[1].parameters.required_status_checks = ["test"]' +malformed topcoder1/nocontext '.[1].parameters.required_status_checks = [{integration_id: 15368}]' +malformed topcoder1/emptycontext '.[1].parameters.required_status_checks[0].context = ""' +malformed topcoder1/numcontext '.[1].parameters.required_status_checks[0].context = 5' +# A good rule does not vouch for a bad one beside it: ci.yml produces the good +# rule's `test`, and the bad rule's checks are unknown. +malformed topcoder1/onebadrule '. + [{type: "required_status_checks", parameters: {}}]' +# A bad check beside a good one in the same list, after it and before it. +malformed topcoder1/badsecondcheck '.[1].parameters.required_status_checks += [{context: ""}]' +malformed topcoder1/badfirstcheck '.[1].parameters.required_status_checks = [{integration_id: 15368}] + .[1].parameters.required_status_checks' +# Parameters that are not an object make the check itself fail in jq, whose +# error must reach neither stream. +malformed topcoder1/stringparams '.[1].parameters = "x"' +# A context that is an object, which the old filter printed into the output. +# shellcheck disable=SC2016 # $c is the edit's jq variable, the canary +malformed topcoder1/objectcontext '.[1].parameters.required_status_checks[0].context = {k: $c}' +# The whole output, byte for byte, is the header and the API error: no +# verdict, and no byte of the answer from any field, canary or not. (cmp, as +# $(...) would drop trailing newlines.) +for r in notype numtype nolist stringlist objectlist stringcheck nocontext emptycontext numcontext \ + objectcontext onebadrule badsecondcheck badfirstcheck stringparams; do + run_validator "topcoder1/$r" + printf '%s\n' "==> Validating required status checks for topcoder1/$r (branch: main)" > "$T/j.out" + printf '%s\n' " API error: GET /repos/topcoder1/$r/rules/branches/main did not answer a valid list of rules" > "$T/j.err" + if [[ $VRC -eq 3 ]] && cmp -s "$T/j.out" "$T/v.out" && cmp -s "$T/j.err" "$T/v.err"; then + pass "j: a rules answer with a malformed rule ($r) exits 3 with the endpoint and nothing else" + else + fail "j: $r should exit 3 with only the API error for its endpoint (got $VRC)" + show "$T/v.out" "$T/v.err" + fi +done + +# Through the fleet: ERRORs that name the endpoint and carry none of the +# answer, so the digest step opens the issue. Before, the run was "All 3 +# repos clean." and opened none. +run_fleet "topcoder1/clean-a topcoder1/notype topcoder1/nolist" +if [[ $FRC -eq 3 ]] && + has "$T/digest.md" "Fleet: 3 repos audited (1 clean, 0 with findings, 2 could not be audited)" && + has "$T/digest.md" "### topcoder1/notype — ERROR: not audited (exit 3: GitHub API error)" && + has "$T/digest.md" "### topcoder1/nolist — ERROR: not audited (exit 3: GitHub API error)" && + has "$T/digest.md" "API error: GET /repos/topcoder1/nolist/rules/branches/main did not answer a valid list of rules" && + ! grep -q "PRIVATE-BODY-CANARY" "$T/digest.md" "$T/f.err" && + [[ "$(needs_issue "$T/digest.md" "$T/clean.md")" == yes ]]; then + pass "j: the fleet files malformed rules answers as ERRORs, and the digest step opens the issue" +else + fail "j: malformed rules answers should be fleet ERRORs that open the issue (got $FRC)" + show "$T/digest.md" "$T/f.err" +fi + +# Still valid: rules of other types with their own parameters, a check with an +# integration_id beside its context, as GitHub answers, and a +# required_status_checks rule whose list is empty, which requires nothing. +fake_repo topcoder1/fullshape "$(rules_json test | jq -c ' + .[1].parameters.required_status_checks[0].integration_id = 15368 + | . + [{type: "pull_request", parameters: {required_approving_review_count: 1, dismiss_stale_reviews_on_push: true}}, + {type: "non_fast_forward"}]')" ci.yml +run_validator topcoder1/fullshape +if [[ $VRC -eq 0 ]] && has "$T/v.out" "✓ test" && ! grep -q "API error" "$T/v.err"; then + pass "j: a full rules answer (other rule types, a check's integration_id) is validated, exit 0" +else + fail "j: fullshape should pass the shape check and validate test, exit 0 (got $VRC)" + show "$T/v.out" "$T/v.err" +fi +fake_repo topcoder1/emptychecks "$(rules_json test | jq -c '.[1].parameters.required_status_checks = []')" ci.yml +run_validator topcoder1/emptychecks +if [[ $VRC -eq 0 ]] && has "$T/v.out" "no required_status_checks rule on main" && + ! grep -q "API error" "$T/v.err"; then + pass "j: a required_status_checks rule with an empty list requires nothing, exit 0" +else + fail "j: an empty list of checks should exit 0 (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 From 7cfa9c00b796f9348bc952e94f802e93f24f5dc8 Mon Sep 17 00:00:00 2001 From: topcoder1 Date: Sat, 17 Oct 2026 14:56:30 -0700 Subject: [PATCH 2/2] fix(audit-required-checks): a rule the filter cannot read is an API error, never no required checks The rules answer's shape check asked only for an array of objects. The contexts filter keeps the rules whose type is required_status_checks and reads their checks with `[]?`, so a malformed rule read as no required checks: exit 0, filed by the fleet audit as clean, no issue opened, and the check the rule required was never validated. Port dotclaude's check (topcoder1/dotclaude#521) verbatim: every rule needs a string type, and every required_status_checks rule a list of objects with a non-empty string context. Anything else exits 3 naming the endpoint. Unlike dotclaude's copy, the error quotes none of the answer: the fleet digest is a public issue (#299). The message takes dotclaude's wording, "did not answer a valid list of rules", since "an array of rules" was wrong for an array holding a bad rule. Case j (d1df485) now passes: 87 of 87 under /bin/bash 3.2.57 and bash 5.3.15. All 13 fleet repos' live rules answers pass the new check, and a full run on this repo gives byte-identical output to the old validator. Co-Authored-By: Claude Opus 5.5 --- scripts/validate-required-checks.sh | 21 +++++++++++++++------ selftest/README.md | 13 ++++++++++++- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/scripts/validate-required-checks.sh b/scripts/validate-required-checks.sh index bb461b8..9782e52 100755 --- a/scripts/validate-required-checks.sh +++ b/scripts/validate-required-checks.sh @@ -164,13 +164,22 @@ uri() { # 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. 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. +# An array of rule objects, each with a string type, whose +# required_status_checks rules each list their checks as objects with a +# non-empty string context, or exit 3: the filter below must never run on +# anything else. A rule missing its type or its list would read as no +# required checks, a clean pass. 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 + ! jq -e ' + type == "array" and all(.[]; type == "object" and (.type | type) == "string") + and all(.[] | select(.type == "required_status_checks"); + (.parameters.required_status_checks | type) == "array" + and all(.parameters.required_status_checks[]; + type == "object" and (.context | type) == "string" and .context != "")) + ' >/dev/null 2>&1 <<< "$RULESET_DATA"; then + echo " API error: GET $RULES_ENDPOINT did not answer a valid list of rules" >&2 exit 3 fi CONTEXTS=$(echo "$RULESET_DATA" | jq -r \ diff --git a/selftest/README.md b/selftest/README.md index 0ac03b9..3f06ef5 100644 --- a/selftest/README.md +++ b/selftest/README.md @@ -285,7 +285,18 @@ arbitrary helper scripts; that's a different kind of repo. 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. + that on the unpaginated, unencoded requests. A third gap, closed the same + day, was a silent pass too: the rules answer was checked only for being an + array of objects, so a rule with no string `type`, a + `required_status_checks` rule whose checks were missing, a string or an + empty object, or a check with an empty `context` read as no required + checks, exit 0, and the fleet filed the repo as clean and opened no issue; + a `context` that was an object was printed, the answer's own bytes, into + the public digest. Every rule now needs a string `type`, and + every `required_status_checks` rule a list of objects with a non-empty + string `context`, as dotclaude's copy has required since its #521; any + other answer exits 3 naming the endpoint and none of the answer, and the + fleet files it as an ERROR. - `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.