[topcoder1]

Summary

This copy of scripts/validate-required-checks.sh checked the rules answer (GET /repos/<repo>/rules/branches/<branch>, merged across pages since #303) only for being an array of objects. The weekly "Audit required checks" workflow runs this copy across the fleet.

The contexts filter keeps only the rules whose type is required_status_checks, and reads their checks with []?. So a malformed rule read as no required checks: exit 0 with no required_status_checks rule on main. The fleet audit then filed the repo as clean and opened no issue, so whatever check that rule required was never validated: a silent pass.

topcoder1/dotclaude#521 (merged 2026-10-16) closed the same gap in dotclaude's copy. This PR ports that check.

What changed

• The shape check (scripts/validate-required-checks.sh) is now dotclaude's jq program, byte for byte:

  • every rule has a string type;
  • every required_status_checks rule lists its checks as an array of objects, each with a non-empty string context.

  Anything else exits 3 and names the endpoint as it was sent. [], when no rule applies, still exits 0.

• fix(audit-required-checks): fail loudly on API errors, file findings apart #299's contract is unchanged:

  • 0 clean, 1 FINDING only, 2 usage, 3 API error, 4 internal error;
  • no message quotes the response, because the digest is a PUBLIC issue. dotclaude's copy appends ${RULES_ANSWER:0:200} to this error; this one names only the endpoint.

• The message is now dotclaude's: did not answer a valid list of rules. "an array of rules" was wrong for an array that holds a bad rule. Four existing assertions take the new wording.

• The CONTEXTS filter is unchanged. Once the check holds, its []? cannot drop anything.

• selftest/README.md: one added passage.

Tests

All tests are in selftest/test_audit_required_checks.sh, new case j. CI runs that file through selftest/test_workflow_guards.py, and every expected value is hardcoded.

The fixtures. Each malformed answer is clean-a's rules answer with one defect, and every ruleset_source in it is a canary. There are 14:

• no type, and a number for it;
• a required_status_checks rule whose checks are missing (parameters: {}), a string, or an empty object, and one whose parameters is a string;
• a check that is a string;
• a check whose context is missing, empty, a number, or an object;
• a bad check after a good one, and before one, in the same list;
• a bad rule beside a good one.

The assertions:

• Each must exit 3, with stdout and stderr byte-equal (cmp) to the header and the one API error line. So no verdict line and no byte of the answer can appear, whichever field it comes from.
• A fleet run over clean-a and two malformed repos must exit 3 with two ERROR: not audited (exit 3: GitHub API error) blocks naming the endpoint, and no canary. The workflow's own digest step must then open the issue.
• Two positive controls, which must still exit 0: a full answer (pull_request and non_fast_forward rules, a check with integration_id), and a rule with an empty list of checks.
• The fixture builder fails the run if an edit does not build a JSON array that carries the canary. Without that, a typo would serve an empty answer, which exits 3 with the same message and passes having tested nothing.

RED first. Commit 83da44c is the tests alone. Against origin/main's validator (5b880ca; the validator is unchanged since 67e3588), 21 of 87 checks fail, the same under /bin/bash 3.2.57 and bash 5.3.15:

• 6 are the new wording.
• 15 are case j:
  • 9 silent passes, exit 0: no type, numeric type, no list, string list, empty-object list, empty context, bad rule beside a good one, bad check after a good one, and the fleet run reading All 3 repos clean.;
  • 4 false findings, exit 1: phantoms named null and 5, a bad check before a good one (null), and an object context, which printed the answer's own bytes into the output;
  • 2 crashes, exit 4: a check that is a string, and string parameters.

GREEN. 87 of 87 pass under both bashes, with identical verdict lines.

• The full suite (uv run pytest -q) gave 787 passed, 1 skipped. It ran before the rebase onto fix(claude-review): skip the review only while the PR is still a draft #304, which adds only fix(claude-review): skip the review only while the PR is still a draft #304's own files. This test's CI wrapper passes on the rebased head.
• shellcheck 0.11.0 is clean on the selftest at style severity, with one SC2016 disable for the jq variable $c.
• The validator reports only the 3 notes origin/main already has.
• prettier 3.9.10 is clean on the README.

Mutation check. Each of these, applied to a scratch copy, makes a named case fail:

• dropping the type, array, string-context or non-empty check;
• the old check;
• checking only the first check of a list, or only the last;
• the shape check's stderr kept, or its redirect order swapped;
• echoing 200 bytes of the answer;
• leaking the rules' parameters, which hold no canary;
• a newline-only leak;
• the old wording;
• exit 0 on a bad shape.

The one survivor, dropping the per-check type == "object", is equivalent: a non-object check makes jq error or evaluate to false, and both take the same exit-3 branch. The independent review confirmed this over 14 check values.

Measured live (read-only)

• Fleet: the real rules answers of all 13 repos the audit covers pass both the old and the new check.
• This repo, end to end: the old validator (67e3588) and the new one both exit 0, with byte-identical stdout (23 lines), in default and --strict mode.

Codex pre-review

• Rounds 1 and 2: native codex review --base. No findings.
• Rounds 3 to 6: an adversarial prompt through codex exec -s read-only. Each round listed the findings already declined, so that only new ones counted.
  • Round 3: 2 P1 and 2 P2.
    • The P1s (declined here): a context with leading or trailing whitespace, or a newline, is altered by the newline-delimited CONTEXTS transport and Python's .strip(). " test " validates as test, and "\n" reads as no rule, exit 0. Both copies behave the same, and the gap predates this change. The right fix is to carry contexts exactly and report a misconfigured context as a finding, not to reject it here as an API error. That is a follow-up, below.
    • P2: case j had no whitespace or newline fixtures; these go with that follow-up.
    • P2: the leak check only grepped for a canary. FIXED with the exact-output assertion.
  • Round 4: 1 P2. $(cat ...) drops trailing newlines, so a newline-only leak would pass. FIXED with cmp.
  • Round 5: 1 P1, a NUL variant of round 3's transport gap. "\u0000" reads as no rule, exit 0. Declined here and folded into the same follow-up.
  • Round 6: No new findings.

Independent review

An opus code-reviewer, separate from the authoring context, reviewed the first draft. Verdict: APPROVE WITH NITS.

• It found no validator defect.
• It re-ran RED and GREEN on both bashes, and every claimed mutant kill.
• It confirmed that the jq program and its comment are byte-identical to dotclaude main, apart from the dropped byte echo.
• It found 5 issues, all in the tests and docs. All are fixed in the tests above:
  • MEDIUM: every bad check was alone in its list, so a first-only or last-only check passed. Two mixed-list fixtures now catch both.
  • LOW: no fixture made the check itself error, so the stderr redirect went untested. A string parameters now does.
  • LOW: a broken fixture edit would pass vacuously. Now guarded.
  • LOW: the README and the case-j comment missed the empty-context silent pass, and said "object" where only {} read as no checks.
  • Note: an object context printed the answer's bytes under the old code. Now pinned.

Re-check of the final tests: APPROVE. It reviewed 8857416, whose files are byte-identical to this branch's.

• It re-ran GREEN and RED under both bashes, in a clean environment (env -i, LANG=C and C.UTF-8), and through CI's entry point. That is pytest selftest/test_workflow_guards.py -k audit_required_checks: 1 passed, and 1 failed on the old validator.
• It killed all 20 non-equivalent mutants, including four leaks no canary can see: the first 20 bytes, a rule's type, the body length, and a trailing space.
• Its one LOW is optional and outside this diff. Cases a, h and i still check their merge-step errors with canary greps, not byte for byte. While that error branch is shared with the shape check, case j guards it.

Preflight

bb-preflight.sh topcoder1/ci-workflows claude/validator-strict-rules-shape was run before the push.

• Before the rebase, it reported NOT READY on two checks:
  • the in-flight cap;
  • a selftest/README.md overlap with fix(claude-review): skip the review only while the PR is still a draft #304, in another entry. A 3-way merge from fix(claude-review): skip the review only while the PR is still a draft #304's real merge base gave 0 conflicts in either order.
• After fix(claude-review): skip the review only while the PR is still a draft #304 merged, this branch was rebased onto 5b880ca, and RED and GREEN were re-run on the rebased commits. bb-preflight now reports NOT READY on the in-flight cap only. chore(deps): bump claude-code-action to v1.0.250, pin --permission-mode default #302 (a draft) and fix(adversarial): leave rg out of the review lane's allowlist #269 are open, and this branch overlaps neither ("no file overlap with sibling Claude PRs").
• Every other check is green: working tree, policy gate, required checks reachable, and ruff.

Known limitations, not changed here

• Contexts the text transport cannot carry exactly: leading or trailing whitespace, newlines, NUL bytes. See the Codex rounds above. Both copies need the same fix, proposed as its own change.
• A non-string ruleset_source_type on a required_status_checks rule makes the filter's ascii_downcase error, so the run exits 4 ("validator crashed") instead of 3. That is loud, not silent. dotclaude's copy behaves the same.
• The rules endpoint lists rulesets only. A check required only by classic branch protection is not seen. No fleet repo requires a check that way alone today.

Codex rounds: 6, the cap. Rounds 1 and 2 were native reviews; rounds 3 to 6 were adversarial. Round 6 found nothing new; round 5's only finding was a variant of round 3's declined transport gap. So the loop stopped at the cap, not on two clean rounds in a row. Of the 6 findings, 2 were fixed (both test gaps, each now killing a mutant) and 4 were declined into the follow-up: the transport gap and its test coverage.

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 analysis is complete. The jq validation filter has been correctly tightened on all dimensions:

• The outer all requiring (.type | type) == "string" catches notype and numtype before the inner all runs on required_status_checks rules
• (.parameters.required_status_checks | type) == "array" catches nolist, stringlist, objectlist, and the case where .parameters itself is a string (jq returns null for field access on a string/null, and null | type == "array" is false)
• all(.parameters.required_status_checks[]; type == "object" and (.context | type) == "string" and .context != "") covers stringcheck, nocontext, emptycontext, numcontext, objectcontext, badsecondcheck, badfirstcheck
• Empty list [] passes vacuously — all(.[]; ...) on an empty array is true in jq, matching the emptychecks test expectation
• The malformed helper's canary + jq -e contains check prevents false-passing tests where a broken jq edit silently produces an empty answer

No issues found. The fix closes a real silent-pass vulnerability and the new test suite (14 malformed cases + fleet integration test + 2 valid-shape acceptance tests) covers all branches.

No issues found. Comprehensive tests for a well-targeted fix that closes a silent-pass bug where malformed rule responses were treated as "no required checks."

_{Claude Review · workflow run}