[topcoder1]

Summary

This closes two gaps in this repo's copy of scripts/validate-required-checks.sh. topcoder1/dotclaude#525 (merged 2026-10-16) closed the same two in dotclaude's copy. This is the copy that the weekly "Audit required checks" workflow runs across the fleet, so each gap was a silent pass that could hide a phantom required check in any fleet repo.

1. The rules call read only one page. GET /repos/<repo>/rules/branches/<branch> returns its rules 30 to a page, and the call had no --paginate. A required_status_checks rule after rule 30 was never read. The branch then passed with no required_status_checks rule on main and exit 0, even when a required context had no producer.

2. --branch and workflow file names went into the URL unencoded. gh parses the endpoint as a URL:

   • it never sends anything after a #;
   • a ? starts the query;
   • a % that starts no %XX escape fails inside gh;
   • GitHub decodes any %XX that is left.

   As a result, --branch 'release#2' read release's rules, and a workflow named ci#2.yml was read as ci. This copy has no --ref.

What changed

• Paging: the rules call runs gh api --paginate.
  • Every page gh prints must be a JSON array. add would treat a null page as nothing at all, so a null page would be merged away unseen.
  • The pages are then merged (jq -cs 'if all(.[]; type == "array") then add else error(…) end'), and the existing shape check runs on the merged list.
  • Both output forms are handled: gh 2.97.0 prints the pages as one array, and gh's help describes one array per page.
  • The shape check needs the merged list because jq goes by the last of several values. That holds for jq -e, and also for jq's exit status: jq 1.7.1, measured 2026-10-17, still exits 0 after an error on an earlier input. Without the merge, a bad rule on page 1 of 2, printed as an array per page, read as a clean pass.
• Encoding: a new uri() (jq @uri, with / left as it is) encodes --branch in the rules path and each workflow file name in the per-file path.
  • Values made only of letters, digits, -._~ and / send exactly the bytes they did before. Any other character is now sent percent-encoded, and GitHub decodes it once, back to the same value.
  • Both call sites are assignments. A uri failure there exits 4; inside a command's arguments it would not stop the command.
• Error messages name the endpoint as sent. This covers the rules shape error and the per-file "did not answer base64 file content" error. api_get's own messages already did.
• Unchanged:
  • fix(audit-required-checks): fail loudly on API errors, file findings apart #299's exit codes: 0 clean, 1 FINDING only, 2 usage, 3 API error naming the endpoint, 4 internal error.
  • fix(audit-required-checks): fail loudly on API errors, file findings apart #299's rule that no message quotes the response, because the digest is a PUBLIC issue. dotclaude's copy quotes up to 200 bytes of a bad rules answer; this one names only the endpoint. The merge therefore reads $API_BODY directly, and no raw answer is kept for a message.

Measured live (gh 2.97.0)

I re-measured these on 2026-10-17 with read-only GETs on this repo. The selftest shim copies them:

• rules/branches/main#x read main's rules.
• rules/branches/ma%69n read main's rules: GitHub decodes %XX.
• contents/a%zz.md failed inside gh with invalid URL escape "%zz", rc 1, and sent nothing.
• --paginate with ?per_page=1 printed this repo's 4 rules as one array (gh followed Link 4 times).

The local-server results come from dotclaude#525 (2026-10-16):

• a null page 2 printed [{"type":"deletion"}null, rc 0;
• a 403 on page 2 printed page 1 and then the error body, rc 1.

Old against new, live

Old is origin/main at 71322c5; new is this branch. Both ran on this repo, topcoder1/ci-workflows, which is itself in the fleet list, through a gh wrapper that logged every call's argv.

Run	Old	New
default	exit 0, 6 contexts ✓	exit 0. Stdout is byte-identical (23 lines). The 41 gh calls are the same, except that the rules call adds --paginate
--strict	exit 0	the same
--branch 'main#x'	exit 0: validated main's 6 contexts	exit 0: no required_status_checks rule on main#x. One call, to rules/branches/main%23x

This repo has 4 rule entries, so paging changes nothing here live. The selftest covers paging. A run on a private fleet repo gave the same verdicts.

Tests

All tests are in selftest/test_audit_required_checks.sh, which CI runs through selftest/test_workflow_guards.py. Every expected value is hardcoded.

The PATH-shim gh reads an endpoint the way gh and GitHub did above:

• It first fills gh's placeholders, {owner}/{repo}/{branch} and :owner/:repo/:branch, as for the scheduled audit's checkout of this repo on main. gh fills them from the checkout it runs in: measured 2026-10-17, contents/{repo}.yml and contents/:repo.yml both requested contents/ci-workflows.yml.
• It drops the fragment, refuses a bad % in the path, and decodes %XX.
• In the query, it splits on &. A parameter with a bad % gets HTTP 400 from the rules endpoint (measured 2026-10-17); the contents endpoint drops it instead.
• It serves rules per branch (branch-rules/).
• It pages rules 30 at a time, or 100 under --paginate, printed as one array or (pages_apart) as one array per page.
• It can also serve GitHub's own pages (rule-pages/), a failing page included. Without --paginate it prints the one page as GitHub sent it. With --paginate it joins them as gh 2.97.0's paginatedArrayReader does: a page with another after it loses its ], and a later page's [ becomes ,.

New cases:

• h (paging):

  • the required-checks rule as rule 31 and as rule 101 (past even a 100-rule page), each in both --paginate forms;
  • a bad rule on page 1 of 2, with pages printed apart;
  • a null page 2, in both forms;
  • a page 2 that fails with 403, in both forms.

  The bad-rule, null and failing-page cases carry canaries, which must not appear in the output.

• i (encoding):

  • workflow file names ci#2.yml, ci?2.yml, 50%.yml, {repo}.yml, :repo.yml and ci 2.yml;
  • --branch release#2 and rel%41se;
  • the endpoint each error names: a failed file read, a file answer that is not base64, and a rules answer that is not a list for rel/main#x. The last one also pins that / is sent as it is.

• s1–s4 (shim controls): these pin what the shim does with the requests origin/main sends (no --paginate, values as given) and with both --paginate forms. They keep cases h and i able to tell the fix from the bug.

RED first. Commit c11a330 is the tests alone: run against origin/main's validator, 15 of its 65 checks fail. The final test file, with the cases the reviews added, fails 18 of 70 there. Both counts are the same under /bin/bash 3.2.57 and bash 5.3.15, and each failure is the reported bug:

• Paging: 4 of 4 rule-31/101 cases exited 0 with no required_status_checks rule on main. The null page 2 exited 1 in both forms, a false FINDING from page 1 judged alone. The failing page 2 exited 0 in both forms, a silent pass.
• File names: # and ? read ci and got a 404. %. failed inside gh. {repo} and :repo read ci-workflows.yml and got a 404.
• Branches: both read a branch with no rules, exit 0.
• Endpoint messages: 3 of 3 named the unencoded endpoint, or exited 0.

The 52 that pass on the old code:

• the 39 existing checks;
• the 11 shim-control checks;
• the space in a file name (gh always escaped it itself);
• the bad rule on page 1, which the old code also rejected, since it read page 1 only. This case kills a fix that paginates without merging.

GREEN: 70 of 70 checks pass under both bashes, and the pytest wrapper for this file passes on a349673. The full suite (uv run pytest -q) gave 787 passed, 1 skipped on 0a15680; the commits since then change only this selftest and its README. shellcheck 0.11.0 is clean at CI's error severity. At default severity the selftest is clean, and the validator reports only the 3 notes origin/main already has (SC2001, SC2034, SC2086). prettier 3.9.10 is clean on the README.

Mutation check (mine; the independent review repeated it). Each of the following, applied to a scratch copy, makes a named case fail:

• no --paginate;
• per_page=100 instead of --paginate;
• --paginate with no merge;
• add with no array guard;
• no uri on the branch;
• no uri on the file name;
• uri that also encodes /;
• an encoder that handles only #;
• an encoder that handles #, ?, % and space but not braces, or not :;
• either message naming the raw value;
• the rules message quoting the answer, which the canaries catch;
• api_get echoing gh's stdout on failure. Only the failing-page-2 canary catches this one.

Codex pre-review

• Round 1: native codex review --base origin/main: no findings.
• Rounds 2–6: an adversarial prompt through codex exec -s read-only. Each round listed the findings already declined, so that only new issues counted.
• Round 2: 2 P2 and 1 P3, all declined:
  • P2: a single page whose body is [][] merges to [], exit 0. The old shape check accepted it too, because jq goes by the last value. It needs GitHub to send invalid JSON with HTTP 200, and it cannot be told apart from gh's array-per-page output, which must stay accepted.
  • P2: NUL bytes in a response are dropped by api_get's command substitution. That capture predates this change (fix(audit-required-checks): fail loudly on API errors, file findings apart #299), and the input needs a raw NUL inside GitHub's JSON.
  • P3: a newline in a workflow file name. This predates the change and is listed below.
• Round 3: 1 P3, and it was false. It claimed the selftest's ${3%]} strips nothing. It did strip the ] under both bashes: the page file it wrote had none. Since 49d23c2 that helper strips nothing, because the fixtures now hold GitHub's own pages and the shim joins them.
• Round 4: 1 P2 and 1 P3, both fixed in db68825, tests only:
  • P2: the failing-page-2 case had no canary, so an api_get that echoed gh's stdout (page 1's rules) would have passed. Page 1 now carries a canary, and this is the only case that catches that echo.
  • P3: gh fills {owner}, {repo} and {branch} from the checkout it runs in. I measured this, the shim now models it, and a {repo}.yml case covers it.
• Round 5: 1 P2 declined and 1 P3 fixed in d7ee6e5:
  • P2: gh 2.97.0's paginatedArrayReader holds back a page's ] only when it is the last byte read, so a page body ending in whitespace would merge into invalid JSON. GitHub's rules pages end in ]: a live 4-page --paginate of this repo's rules merged into valid JSON. If a page did not, the validator would exit 3 naming the endpoint, which is the fail-closed path.
  • P3: gh also fills :owner, :repo and :branch (its placeholderRE). I measured this and added a :repo.yml case.
• Round 6, on d7ee6e5: No findings.

Independent review

An opus code-reviewer lane, separate from the authoring context, reviewed 71322c5..d7ee6e5. Verdict: APPROVE with nits. It found no validator defect at any confidence.

• uri failure: it made uri fail at each call site, on both bashes. Both sites exit 4, and no request goes out with a truncated endpoint. With uri inlined into an argument instead, the request goes out as .../workflows/ and the run exits 3. That is why both call sites are assignments.
• The merge, against real gh 2.97.0 output from a throwaway local server: one merged array, [...null, page 1 followed by the 403 body, and an empty last page.
• The merge, against real GitHub: per_page=100, the Link-following, and 4 pages at per_page=1 printed as one array.
• Leaks: both the array guard and the merge's 2>/dev/null are needed. jq's add type error quotes the input.
• Mutations: 24, all killed on both bashes. That covers the ten listed above, plus eleven of its own: any for all in the guard, % or ? kept, the shape check dropped or run on the raw answer, the merge's stderr kept, uri inlined, and more. The three that survived earlier commits are the ones db68825 and d7ee6e5 closed: api_get echoing gh's stdout, braces left unencoded, and : left unencoded.
• RED/GREEN: it reproduced the numbers at each commit, with identical verdict lines on both bashes.
• Its two LOW findings were both on shim fidelity. It measured both against real GitHub and gh, and both are fixed in 49d23c2:
  • A bad % in the rules endpoint's query gets HTTP 400; only contents drops the parameter. s1 had asserted the drop.
  • The page fixtures had held gh's --paginate bytes. The old, unpaginated validator therefore saw an unclosed page 1 and looked fail-closed, when it really exited 1 (null page) and 0 (failing page 2). rule-pages/ now holds GitHub's own pages, and the shim joins them as gh's paginatedArrayReader does.
• Re-check of 49d23c2: APPROVE.
  • Real gh 2.97.0 and the shim, given the same page bodies, printed byte-identical output in 24 of 26 scenario pairs. Those included an empty middle page, an empty first page and a null first page.
  • The other 2 differ only by a trailing newline after a failing page's error body, which comes from the existing fail_with echo and affects no outcome.
  • The 400 is byte-exact against real gh.
  • Its optional nit, an empty-page join that nothing tested, is pinned in a349673. The shim mutation that joined it as , is now caught.
• Left below the bar:
  • Next-page Links for an encoded branch. This would matter only past 100 rules, on a branch with special characters, through a manual --branch.
  • Pre-existing: this copy's rules shape check is weaker than dotclaude's. It requires no string type and no non-empty string context, so a malformed rule of that kind reads as no required checks, exit 0. That is out of scope here; a follow-up session is proposed.

Preflight

bb-preflight.sh topcoder1/ci-workflows claude/validator-page-encode:

• On 0a15680 it reported READY TO PUSH.
• On f0fad3b, 49d23c2 and a349673 it reported NOT READY on the in-flight cap only. Two other Claude PRs are open: chore(deps): bump claude-code-action to v1.0.250, pin --permission-mode default #302, a draft opened 2026-10-17, and fix(adversarial): leave rg out of the review lane's allowlist #269. Neither touches these three files, and the preflight's file-overlap check is green.
• Every other check is green: working tree, policy gate, required checks reachable, and ruff.

Known limitations, not changed here

• A newline inside a workflow file name splits it into two names in the listing loop. This predates the change; dotclaude#525 lists the same limitation.
• A failure on page 2 or later is reported against the endpoint the validator called, not the page URL gh followed. gh's stderr never carries that URL.
• The fleet wrapper always audits main. All 14 fleet repos default to main today (checked 2026-10-17), so a repo added later with another default branch would read as clean.
• The quadratic gsub("\\s"; "") in the per-file base64 check is unchanged.

Codex rounds: 6, the cap. Round 1 was the native review; rounds 2–6 used the adversarial lane. Round 6, on d7ee6e5, found nothing; the commits after it (README wording, plus the independent review's test-only fixes) were re-checked by the reviewer, since the loop was at its cap. Round 5 did find something, so the loop stopped at the cap, not on two clean rounds in a row. Of the 8 findings, 3 were fixed (all test gaps, each test failing first), 4 were declined for the reasons above, and 1 was false.

Auto-merge rationale: manual click-merge. PRs to topcoder1/ci-workflows are always manual merge, and this validator decides the fleet audit's verdicts, which are posted publicly.

🤖 Generated with Claude Code

[github-actions]

The merging logic works correctly for both gh output forms. The analysis is complete:

uri(): jq @uri percent-encodes all characters except what jq considers safe, then split("%2F") | join("/") restores forward slashes. The comment correctly notes to call it in an assignment context so a jq failure propagates rather than being silently swallowed inside a command argument.

Pagination (jq -cs): For gh 2.97.0's merged single array, jq -cs wraps it in [[...all rules...]], then all(.[]; type == "array") passes and add flattens it back. For per-page arrays, jq -cs produces [[page1], [page2]], which add flattens correctly. A null page 2 in the per-page form adds null to the outer array, failing all(.[]; type == "array") and triggering error(). A null page in the gh 2.97.0 form produces invalid JSON, making jq -cs fail at parse time. Both paths correctly exit 3.

Shim fail file vs path routing: The fail file is matched against rest (percent-encoded, with query string), while routing uses path (percent-decoded, path only). The test correctly uses ci%232.yml in the fail file, matching the encoded rest value. Workflow file lookups (answers/$name, workflows/$name) use path-derived name (decoded), matching the actual filenames on disk.

Branch lookup in branch-rules/: The branch variable is set from path (decoded), so rel%41se is stored/looked up as relAse and release#2 as release#2 — matching the test's directory names.

No logic bugs found. The test coverage for the new code paths is thorough.

No issues found. Diff correctly fixes two silent-pass bugs (unpaginated rules endpoint and unencoded URL values) with well-matched test coverage for both forms of gh --paginate output and all the URL-special characters.

_{Claude Review · workflow run}