Repository navigation
fix(tag-ruleset-canon): enumerate and probe through the App installation - #1210
Merged
Merged
Conversation
tag-ruleset-canon.yml now mints one App installation token per owner (RULESET_APP_ID, #1208). The applier still assumed a PAT, so both steps would have failed on their first App run: - hyperpolymath step: user/repos needs a user identity and answers an installation token (ghs_) with 403, so the run died under set -e before examining a single repository. Under an installation token the applier now lists installation/repositories instead, private repos included, archived ones dropped. A failed listing dies naming the endpoint. - metadatastician step: the apply-mode write probe always self-PUT GITHUB_REPOSITORY (hyperpolymath/standards), which an installation on metadatastician cannot write, so a capable credential got a false exit 3. The probe now self-PUTs a repo-level tag ruleset inside the installation, skipping org-inherited ones (a per-repo PUT of those 404s). A credential that cannot write still stops the run with exit 3. - ESTATE_ORGS='' (set by the hyperpolymath step) fell back to metadatastician through ${ESTATE_ORGS:-...}, so that step also swept repos its token cannot write. An explicit empty value now means no orgs. PAT behaviour is unchanged: user/repos and the GITHUB_REPOSITORY probe. scripts/tests/apply-tag-ruleset-canon-test.sh runs the applier against a stub gh that refuses what GitHub refuses (planted positives), and four mutants reintroduce each defect; the origin/main applier fails 5 of its cases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015bTuGfwCcvjrmNFejydTML
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 26 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
K9 contract conformancerun https://github.com/hyperpolymath/standards/actions/runs/37861614231 K9 normative contract typecheckK9 contract self-testK9 conformance fixturesK9 corpus conformance (L2) |
|
7 of 13 tasks
hyperpolymath
added a commit
that referenced
this pull request
Oct 9, 2026
…1211) ## Summary `scripts/apply-workflow-pins-remote.sh` lists its targets with `users/<owner>/repos`, falling back to `orgs/<owner>/repos`. Both endpoints answer a GitHub App installation token (`ghs_…`) with **public repositories only**. So once `APP_ID` is set, the private repositories the applier App is installed on, and can write, are **never audited and never re-pointed**. This PR adds them. 1. **Under a `ghs_` token, the installation's own repositories are added to the public listings.** They come from `installation/repositories`, with archived ones dropped and the list filtered to `--owners`. They are added, not substituted: `GITHUB_TOKEN` is `ghs_` too, and its installation is this one repository. Replacing the listing would shrink the scheduled audit census, which runs on `GITHUB_TOKEN` while no App is configured, to `hyperpolymath/standards` alone. 2. **Enumeration now fails closed.** `list_repos` returns 1 and prints nothing when the installation listing fails, or when an owner's `users/` and `orgs/` listings both fail. `main` then stops before writing a census and says the enumeration failed. Before this, the second listing's stderr went to `/dev/null` and its exit status was ignored, so a rate-limited owner silently contributed zero repositories. That is the same fail-open `fetch_workflows` was cured of on 2026-10-02. **Under a PAT nothing changes:** the public listings, and `installation/repositories` is never called. The workflow file is **not** touched. No tracking issue. This is the bug-fix half of the owner's request "push the f88b721 with bug fix" (dev-notes `f88b721` recorded the finding). ## Type of change - [x] 🐛 Bug fix (non-breaking change that fixes an issue). Under an App token the census now includes the installation's private repositories. Under a PAT or `GITHUB_TOKEN` the public census is unchanged. - [ ] ✨ New feature. No new capability; the App path now sees what the App can write. - [ ] 💥 Breaking change. One behaviour changes on purpose: a run whose enumeration fails now exits 1 instead of reporting a partial census. That is the intended fail-closed behaviour, not a break of any caller. - [ ] 🕳️ Soundness fix. The classifier and the rewrite logic are untouched. The fail-closed enumeration does remove a false "nothing to do" for a rate-limited owner, but it is listed under Bug fix. - [ ] 📖 Documentation. Only `list_repos`'s own docstring is new. - [ ] 🧹 Refactor / tech debt. Not behaviour-preserving (see Bug fix). - [ ] ⚡ Performance. Under an App token there is one extra paginated listing per run; under any other token there is none. - [x] 🔧 Build / CI / tooling. An estate maintenance script and its test suite. ## 📌 New pins - **Head SHA: `1ba3f5b0670c475506db73e214da20058d59191f`** - None. No `uses:` SHA, `actions.lock` entry, lockfile record or container digest is added or changed. No workflow file is touched. ## How has this been verified? All local commands were run in the worktree on Debian 13 (WSL2). - **`bash tests/test_apply_workflow_pins_remote.sh` → 41 PASS, `RESULT: all checks passed`, rc 0.** The new section 4 runs `list_repos`, and `main` end to end, against a stub `gh` that serves listing fixtures with real `jq` and refuses `installation/repositories` to a non-`ghs_` token. - **Planted positives (2):** the stub refuses `installation/repositories` to a PAT, and fails a flagged `users/` listing. So no fail-closed case can pass because nothing ever failed. - **Cases (9):** - App token: exactly `hyperpolymath/priv-b`, `hyperpolymath/pub-a` and `metadatastician/pub-c`. The private installation repo is included, both archived repos are dropped, and `pub-a` (public and in the installation) appears once. - `GITHUB_TOKEN`-shaped `ghs_` token whose installation is `hyperpolymath/standards`: the public census survives alongside it. - PAT: the public listings only, and `installation/repositories` is absent from the call log. - `--owners metadatastician` under an App token on hyperpolymath: no hyperpolymath repo leaks in. - A `users/` 404 falls back to `orgs/`. - Failed installation listing: rc 1, nothing printed, and the endpoint is named. - Both public listings fail for one owner: rc 1, nothing printed, and the owner is named. - `main`, App token: the census row `hyperpolymath/priv-b ci.yml FRESH` is present. - `main`, failed enumeration: rc 1, no census, "repository enumeration failed" on stderr. - **Mutants (6).** Each one is checked to have changed the file and to parse (`bash -n`), and each is killed: - `ghs_` detection disabled; - the installation listing replacing the public one (killed by the `GITHUB_TOKEN` case); - dedupe removed; - the owner filter dropped; - the installation-listing failure swallowed; - the public-listing failure swallowed. - **The `origin/main` applier through the same section 4 → 7 of its 12 behaviour checks fail.** - Under an App token `priv-b` is missing. - A failed installation listing returns rc 0 with the public repos. - A rate-limited owner yields a partial list, which `main` would have accepted. - The end-to-end census lacks the private repo. - A failed enumeration is not reported as one. - **Live audit run on this head, under `GITHUB_TOKEN`:** [run 37865067072](https://github.com/hyperpolymath/standards/actions/runs/37865067072), dispatched with `mode=audit` and `limit=2`. The run succeeded. The cred step logged "AUDIT-ONLY: no App credential; census will run on GITHUB_TOKEN". The applier enumerated with no FATAL, walked 2 repos and reported `BEHIND 4`. - **`shellcheck`** on both files → 11 findings on this branch and 11 on `origin/main`, and the two sets are identical (compared with line numbers stripped). This PR adds none. Two new `SC2016` disables carry a reason: `$1` and `$2` there are expanded by an inner `bash -c`. - **`.githooks/docstring-scan.sh --worktree --check` → `functions=17 documented=17 coverage=100.00%`.** - **`bash scripts/run-shell-test-suite.sh` → 79 of 81 test files pass.** The 2 failures are **pre-existing on `main`**: `scripts/tests/build-registry-test.sh` and `scripts/tests/build-scorecards-test.sh` fail the same way on `900c42c7`. This PR touches neither generator nor either test. - **Pre-commit hooks:** all passed (gitleaks, SPDX, sha-pins, actions-lock, permissions, codeql, bot-directives). - **PR CI on this head** (check-runs read with `--paginate`): 66 runs. 52 success, 12 skipped, 2 failure. - The 3 required contexts passed: `governance / Actions lockfile verify`, `uses ⊆ actions.lock` and `scan / gitleaks`. - In Repo self-tests, `tests/test_apply_workflow_pins_remote.sh` PASSes. The job reports "2 of 82 test file(s) failed": build-registry and the Wave-3 scorecard test, the same two pre-existing failures. The count is 82 because the merge ref includes #1210's new test. - **Code scanning:** on the PR merge ref, CodeQL (actions), CodeQL (javascript-typescript) and Hypatia each analysed `408ef607` and returned 0 results. The open alerts on the PR ref minus those on `main` form the **empty set**. All 10 open alerts on `main` are Scorecard findings, and Scorecard does not run on PRs. **Horizon:** - The stub cases model GitHub's documented behaviour; they are not a live App run. The live run above proves that the `GITHUB_TOKEN` audit path still works on this head. It does **not** show that `installation/repositories` was called, because the log masks the token: that rests on `GITHUB_TOKEN` carrying the documented `ghs_` prefix. - The App path itself gets its live acceptance run once `APP_ID` and `APP_PRIVATE_KEY` are set and the applier App is installed. ## Checklist - [x] My commits are **signed**: one commit, `git log --show-signature` → `G`, ED25519. - [x] I ran the project's own checks/tests locally and they pass, apart from the 2 test files that fail the same way on `main` (see above). - [x] New files carry the correct `SPDX-License-Identifier`. No new files; both changed files keep their existing `MPL-2.0` header, and no file was relicensed. - [x] Docs are updated, and no public claim now overstates what the code does. `list_repos`'s new docstring says what each credential sees and why the listings are unioned. The workflow's comments are unchanged and remain accurate. - [x] I have not introduced a soundness hole. Enumeration is now stricter (fail-closed), and the classifier, the rewrite and `fetch_workflows` are untouched. ## Notes for reviewers - **Why union and not replace?** #1210 could replace `user/repos` with `installation/repositories` because `user/repos` 403s for any installation token. Here `users/<o>/repos` succeeds under `ghs_` and merely under-reports. Replacing it would turn today's `GITHUB_TOKEN` audit into a census of one repository. The `replace_not_union` mutant exists to keep that from regressing. - **Out of scope, recorded in dev-notes `inbox/findings.md`:** - The workflow mints `tok-org` (the applier App scoped to metadatastician) and never uses it; L126 passes only `tok-user || GITHUB_TOKEN`. Even after this PR, metadatastician's **private** repos stay invisible, and `--fix` writes to metadatastician repos fall outside the hyperpolymath installation. - The fix is one applier step per owner, each with its own token. That is a workflow edit, held until this repo's gates read KYAML (AGENTS.md §2a). - A PAT also lists public repos only. That is unchanged here. - **Red check deferred:** `Registry + topology in sync` also fails on current `main`, `2e12b312` (measured with check-runs `--paginate` at PR open). It has been red since `bbcc722b` (#1206). This PR touches neither the registry nor its generator. Tracked by #1161 §2, which has acceptance criteria. - **Red check deferred:** `Repo self-tests` also fails on current `main`, `2e12b312`. The same two files fail on `900c42c7` and on this head: `scripts/tests/build-registry-test.sh` and `scripts/tests/build-scorecards-test.sh`, both from the `REGISTRY.a2ml` drift. Tracked by #1161 §2. Whether to regenerate the registry or retire the check with `.a2ml` is the owner's ruling, so this PR does not regenerate it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_015bTuGfwCcvjrmNFejydTML Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Summary
tag-ruleset-canon.ymlmints one GitHub App installation token per owner (RULESET_APP_ID, #1208).scripts/apply-tag-ruleset-canon.shstill assumed a PAT, so both workflow steps would have failed on their first App run. This PR makes the applier work under an installation token and leaves PAT behaviour unchanged.Three defects are fixed:
user/reposunder an App token. That endpoint needs a user identity, so it answers an installation token (ghs_…) with403 Resource not accessible by integration. Underset -ethe run died before it examined any repository. Under an installation token the applier now listsinstallation/repositories(private repos included, archived ones dropped). If that listing fails, the run dies and names the endpoint.GITHUB_REPOSITORY(hyperpolymath/standards). An installation on metadatastician cannot write that repo, so a capable credential got a false exit 3 ("credential cannot WRITE rulesets"). Under an installation token the probe now self-PUTs a repository-level tag ruleset inside its own installation. It skips org-inherited rulesets, because a per-repo PUT of those 404s (O6 propagation: four contradictions between the ruling and the committed rulesets #1032). A credential that really cannot write still stops the run with exit 3.ESTATE_ORGS=''meant metadatastician. The hyperpolymath step setsESTATE_ORGS: '', but${ESTATE_ORGS:-metadatastician}treated the empty string as unset. That step would therefore also sweep metadatastician repos its token cannot write. It is now${ESTATE_ORGS-metadatastician}: an explicit empty value means no orgs, and the default still applies when the variable is unset.Unchanged under a PAT (the
ESTATE_ADMIN_TOKENfallback): targets come fromuser/reposand the probe self-PUTsGITHUB_REPOSITORY. The workflow file is not touched.No tracking issue. This implements the owner's ruling of 2026-10-08: "Fix in a standards PR now".
Type of change
ESTATE_ORGS=''now means no orgs. The only caller that sets it to''is the hyperpolymath step oftag-ruleset-canon.yml, which wants exactly that.--skip-userandESTATE_ORGSunder an App token.📌 New pins
e39d890c6de1f56faa6bdba1f02aca20234f4aeduses:SHA,actions.lockentry, lockfile record or container digest is added or changed. No workflow file is touched.How has this been verified?
All commands were run in the worktree on Debian 13 (WSL2).
scripts/tests/apply-tag-ruleset-canon-test.sh→passed=14 failed=0. It runs the applier against a stubghthat serves fixtures and refuses what GitHub refuses:ghs_onuser/repos→ 403;ESTATE_ORGS='': targets are exactly the installation's non-archived repos, both CONVERGED, rc 0, anduser/reposis never called.user/repos.GITHUB_REPOSITORY.GITHUB_REPOSITORY=hyperpolymath/standards: exactly one probe PUT, to the repo-level ruleset inside the installation. The org-inherited repo listed first is skipped and reported ORG-INHERITED. rc 0.bash -n-checked and each killed:ghs_*detection disabled, killed by 2 cases;${ESTATE_ORGS:-…}restored;source_type == "Repository"filter removed from the probe;origin/mainapplier through the same harness → 5 cases failed:user/repos403, as was the failed-listing case;ESTATE_ORGS='':metadatastician/deltawas swept;bash tests/test_tag_ruleset_canon.sh→passed=31 failed=0(the existing structural guards, properties 1–14).shellcheckon both files → rc 0. OneSC2016is disabled on a single line, with a reason: the${…}there is sed text to match..githooks/docstring-scan.sh --worktree --check→functions=17 documented=17 coverage=100.00%.bash scripts/run-shell-test-suite.sh→ 80 of 82 test files pass. The 2 failures are pre-existing onmain:scripts/tests/build-registry-test.sh(2 ❌) andscripts/tests/build-scorecards-test.sh(10 ❌) fail identically in a clean detached worktree oforigin/main900c42c7. This PR touches neither generator nor either test.scripts/tests/apply-tag-ruleset-canon-test.sh→passed=14 failed=0. The run reports2 of 82 test file(s) failed, and they are the same two pre-existing files.Horizon: this is a local stub of GitHub's behaviour, not a live run. The live acceptance run is the next step. It needs the ruleset App to be created and installed on both owners, and
RULESET_APP_ID/RULESET_APP_PRIVATE_KEYto be set on this repo. Thentag-ruleset-canonis dispatched withapply=false.Checklist
git log --show-signature→G, ED25519.main(see above).SPDX-License-Identifier: the new test isMPL-2.0. No existing file was relicensed.--skip-userandESTATE_ORGS=''do under an App token.Notes for reviewers
ghs_rather than tryuser/reposand fall back? A 403 fromuser/reposis the same 403 a mis-scoped PAT gets. A fallback would quietly turn a broken PAT into "list whatever installation/repositories says". The token prefix is the one unambiguous signal.GITHUB_TOKENis alsoghs_. Its installation lists only this repo, so in apply mode the write probe PUTs here, gets 403 (noadministration:write) and stops the run with exit 3, as before. In a dry run it would report on this one repo only.list_installation_reposruns at most once. Both the probe and the enumeration need it. It is called in the main shell, never inside$(…), so the once-only flag survives.scripts/apply-workflow-pins-remote.shlists targets withusers/<owner>/repos, which returns public repos only. That is recorded as a finding, not fixed here.Registry + topology in syncalso fails onmainat900c42c7. It has been red sincebbcc722b(chore(deps): bump rustls from 0.23.37 to 0.23.45 in /rhodium-standard-repositories/satellites/rsr-certifier in the cargo group across 1 directory #1206). This PR touches neither the registry nor its generator. Tracked by Two unowned reds on #1160: self-ci Haskell job builds at root with no cabal.project; registry drift on main #1161 §2, which has acceptance criteria.Repo self-testsalso fails onmainat900c42c7(Self Test run 37770790241). The same two files fail there and on this head:scripts/tests/build-registry-test.shandscripts/tests/build-scorecards-test.sh, for the cause in the row above (REGISTRY.a2mldrift). Tracked by Two unowned reds on #1160: self-ci Haskell job builds at root with no cabal.project; registry drift on main #1161 §2. The fix, either regenerating the registry or retiring the check along with.a2ml, is the owner's ruling, so this PR does not regenerate the registry.🤖 Generated with Claude Code
https://claude.ai/code/session_015bTuGfwCcvjrmNFejydTML