diff --git a/.github/workflows/bricked-prs.yml b/.github/workflows/bricked-prs.yml index 0c57043..39539ca 100644 --- a/.github/workflows/bricked-prs.yml +++ b/.github/workflows/bricked-prs.yml @@ -45,6 +45,34 @@ jobs: # one human's PAT. `owner:` yields an ORG-scoped installation token, which is # what an org-wide read needs. No fallback to the PAT: a fallback would let a # broken App path keep working silently. + # + # SCOPES WERE MEASURED, NOT DERIVED (backend#2157, 2026-08-24). This audit was + # exempted from `scripts/mint-scope.py` because the protection endpoints are + # known to DEGRADE rather than error -- a narrower token can answer 200 with + # fewer fields, and a missing field reads as "not configured", which would make + # a protected branch look unprotected on a green run. Reading the docs cannot + # settle that, so it was run: five token scopes against the real endpoints, then + # `scripts/bricked-prs.py` itself over backend + client-runtime + client + + # .github. At the five scopes below the script produces output IDENTICAL to the + # full grant -- 4 findings, `0 COULD NOT AUDIT`, exit 1. + # + # Two of the five are non-obvious and BOTH were verified by removal: + # + # administration: read -- classic protection. Dropping it does not silently + # empty the required set; `branches/{b}/protection` answers 403, which + # read_protection() turns into an error (12 COULD NOT AUDIT, exit 2). It + # also keeps the 404 on an UNPROTECTED branch reading "Branch not + # protected", the fact read_protection() relies on to tell "no classic + # protection" apart from "could not read". + # actions: read -- NOT for any Actions API. `gh pr list --json + # statusCheckRollup` resolves `commit.status` underneath, and without + # actions:read GraphQL refuses that subfield on a PRIVATE repo while every + # other scope here is untouched (3 COULD NOT AUDIT, exit 2). Nothing in the + # docs connects the two; only removing it shows the link. + # + # The measurement drops contents from WRITE to read and removes issues:write and + # organization-projects:write entirely. Do not add them back to "be safe": this + # token reads, and the App holds bypass_reviews fleet-wide. - name: Mint an installation token id: app-token uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 @@ -52,6 +80,11 @@ jobs: app-id: ${{ secrets.RELEASE_TRAIN_APP_ID }} private-key: ${{ secrets.RELEASE_TRAIN_APP_PRIVATE_KEY }} owner: ${{ github.repository_owner }} + permission-administration: read # classic branch protection + permission-actions: read # statusCheckRollup -> commit.status (see above) + permission-checks: read # check-suites, the head-age clock + permission-contents: read # branch list, commit fallback clock + permission-pull-requests: read # the open-PR list and its rollup - name: Audit env: # Needs to read branch protection, rulesets and PRs across the org. diff --git a/.github/workflows/merge-settings-drift.yml b/.github/workflows/merge-settings-drift.yml index 28ae463..9c96d37 100644 --- a/.github/workflows/merge-settings-drift.yml +++ b/.github/workflows/merge-settings-drift.yml @@ -22,9 +22,14 @@ # on merge. With the setting off, no cleanup command can ever reap the branch. # # REPORT-ONLY by design. It never PATCHes a repo: a settings change should be a -# visible, deliberate act, and the token then only needs read access rather than -# standing administration:write over every repo (see backend#1343 on -# over-scoped credentials). The report tells you the one command to run. +# visible, deliberate act, and the token then needs no repo-administration +# permission at all (see backend#1343 on over-scoped credentials). The report +# tells you the one command to run. +# +# It does still hold `contents: write`, which looks like a contradiction and is +# not: GitHub returns the four merge-setting fields only to a caller with PUSH +# access, so that grant buys this job a READ it cannot otherwise perform. Measured +# 2026-08-24 -- the mint step below carries the numbers. name: Merge-settings drift @@ -49,6 +54,37 @@ jobs: # not calls it makes, so no repo-administration permission is needed or asked # for. No fallback to the PAT: a fallback would let a broken App path keep # working silently. + # + # `contents: write` IS LOAD-BEARING AND IT IS NOT A TYPO. Read this before + # "fixing" it (backend#2157, measured 2026-08-24). + # + # `GET /repos/{owner}/{repo}` returns `allow_merge_commit`, + # `allow_squash_merge`, `allow_rebase_merge` and `delete_branch_on_merge` ONLY + # to a caller with PUSH access. For an App installation the push signal is + # `contents: write`. Everything else is a 200 with those four keys simply + # ABSENT -- no error, no warning, nothing in the status line to notice. + # + # Measured, not reasoned, because the shape of the failure is invisible to a + # green run. Six token scopes were run against the real endpoint and then this + # job's own audit loop over all 19 active repos: + # + # contents: write -> 19 readable, 0 unreadable (same as the full grant) + # contents: read -> 0 readable, 19 unreadable + # administration: read, no contents:write -> 0 readable, 19 unreadable + # contents: write, no administration -> 19 readable, 0 unreadable + # + # So `administration: read` is neither necessary NOR sufficient here, which is + # the opposite of what both this file and the report below used to say, and it + # is dropped. What the org listing needs (metadata) is implicit. + # + # This is the one grant backend#2157 set out to delete everywhere, and this is + # the one workflow that measurably cannot lose it. Swapping it to `read` does + # not fail loudly at mint time -- it fails 19 times inside the loop, where the + # non-boolean guard catches it and reddens the run. Loud, but only after the + # fact, and only because that guard exists. + # + # Still a real narrowing: administration:read, actions:read, checks:read, + # pull-requests:write and organization-projects:write all go. - name: Mint an installation token id: app-token uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 @@ -56,6 +92,8 @@ jobs: app-id: ${{ secrets.RELEASE_TRAIN_APP_ID }} private-key: ${{ secrets.RELEASE_TRAIN_APP_PRIVATE_KEY }} owner: ${{ github.repository_owner }} + permission-contents: write # a READ requirement, not a write one -- read the block above + permission-issues: write # the drift report comments on backend#1337 # Needed for repo-inventory.yml, which the listing is cross-checked against # below. The sibling audits (caller-drift, standards-sync) check out for the @@ -154,12 +192,18 @@ jobs: autodelete=$(printf '%s' "$settings" | cut -f4) # FAIL CLOSED on anything that is not a literal true/false. These - # four fields are only returned to a principal with admin on the - # repo, so a token that can LIST but not read settings gets a - # perfectly successful response with the keys absent — jq then - # yields empty strings, which match neither branch below and would - # have been silently counted as compliant. That is the same false - # all-clear the zero-repo guard above exists to prevent. + # four fields are only returned to a principal with PUSH access to + # the repo (`contents: write` on an App installation — measured + # 2026-08-24, backend#2157; it is NOT `administration`, which this + # comment claimed until then), so a token that can LIST but not read + # settings gets a perfectly successful response with the keys absent + # — jq then yields empty strings, which match neither branch below + # and would have been silently counted as compliant. That is the same + # false all-clear the zero-repo guard above exists to prevent. + # + # If every repo lands in `failed(unreadable)` at once, the token lost + # `contents: write`, not `administration: read`. That is the whole + # failure mode, and it looks like a settings problem. for value in "$merge" "$squash" "$rebase" "$autodelete"; do case "$value" in true | false) ;; @@ -279,10 +323,11 @@ jobs: if [ -n "$FAILED" ]; then printf '### Settings could not be read\n\n%s\n\n' "$FAILED" - printf 'These are NOT known to be compliant. The three merge fields are only returned\n' - printf 'to a principal with admin on the repo, so `(unreadable)` usually means the\n' - printf 'token lacks admin there rather than that anything is misconfigured. Fix the\n' - printf 'token permissions (or check the repo still exists), then re-run.\n\n' + printf 'These are NOT known to be compliant. The four merge fields are only returned\n' + printf 'to a principal with PUSH access, so `(unreadable)` usually means the token\n' + printf 'lost `contents: write` rather than that anything is misconfigured — and if\n' + printf 'EVERY repo is listed here, that is certainly what happened (backend#2157).\n' + printf 'Fix the token permissions (or check the repo still exists), then re-run.\n\n' fi printf '[Run log](%s)\n' "$RUN_URL" diff --git a/scripts/mint-scope.py b/scripts/mint-scope.py index 6091073..4ab4b20 100755 --- a/scripts/mint-scope.py +++ b/scripts/mint-scope.py @@ -89,35 +89,46 @@ # reported as a finding (see `stale_exemptions`). # # These rows are the pre-existing state this guard was written to stop growing, not -# to fix in one commit: `fr-gate` is a required check on every promotion branch in -# the fleet, and two of the others read branch protection, where a narrower token -# is known to return LESS data rather than an error (the reason `caller-drift` -# keeps a PAT at all -- ruleset `bypass_actors` is returned only to a write-access -# caller). Downscoping those needs measurement per workflow, which is its own work. +# to fix in one commit. Ten became six (advance-deploy-env, customer-priority-bump, +# fr-pass-comment, kanban-closure-router -- derivable from their call sites), and +# six became four. # -# FOUR ROWS ARE GONE, and what remains is not "the rest of the same job" -# (backend#2157). The four burnt down -- advance-deploy-env, customer-priority-bump, -# fr-pass-comment, kanban-closure-router -- were the ones whose reason was literally -# "scopes not yet measured": each makes a small, closed set of board / issue / PR -# calls, and the scopes are derivable from those call sites plus GitHub's documented -# per-endpoint requirement. The six left are each left for a STATED reason, not for -# lack of time: +# THE TWO JUST BURNT DOWN WERE THE ONES THAT COULD NOT BE DERIVED, and the reason +# they could not is worth keeping (backend#2157, measured 2026-08-24). Their rows +# said a narrower token may return LESS rather than erroring -- a 200 with a field +# missing, which a checker reads as "not configured" on a green run. That was the +# right worry, so it was measured rather than argued: token scopes minted against +# the live endpoints, then each workflow's OWN script run at the candidate scope and +# compared to the full grant, then each permission removed one at a time to prove it +# was load-bearing. What came back was not what either row predicted: +# +# bricked-prs -- branch protection does NOT degrade silently. Without +# `administration: read` it answers 403, which read_protection() reports as an +# error, so the audit fails closed exactly as designed. It narrowed to five READ +# scopes, output identical to the full grant. +# merge-settings-drift -- reads no branch protection at all; the row was wrong +# about which endpoint it even used. It reads `GET /repos/{o}/{r}`, and THAT is +# the endpoint that degrades silently: the four merge-setting fields are +# returned only to a caller with PUSH access, absent otherwise, 200 either way. +# `contents: write` is the one grant it measurably cannot lose -- the same grant +# this ticket set out to delete everywhere. +# +# So the silent-degradation risk was real, but attached to the other workflow. That +# is the argument for measuring rather than reasoning, and it is why the four below +# say what they say. +# +# The four left are left for a STATED reason, not for lack of time: # # fr-gate, set-pr-status, standards-sync, kanban-reconcile -- high exposure # (a required check fleet-wide, a caller in every repo, a contents:write sweep, # the widest board surface). Each needs its own window so one bad scope does not # redden the whole fleet at once. -# bricked-prs, merge-settings-drift -- branch-protection reads, where a narrower -# token SILENTLY RETURNS LESS rather than erroring. That failure mode is -# invisible to a green run, so it needs a measurement, not a derivation. # # The distinction matters because it is the difference between an exemption that is # a to-do and one that is a decision. Do not fold them back together. EXEMPT = { - "bricked-prs.yml": "reads branch protection; a narrower token may return less, not error", "fr-gate.yml": "REQUIRED check on every promotion branch fleet-wide -- needs its own window", "kanban-reconcile.yml": "the weekly backstop -- widest board surface of the set", - "merge-settings-drift.yml": "reads branch protection; same caveat as bricked-prs", "set-pr-status.yml": "runs fleet-wide from every repo; largest exposure of the set", "standards-sync.yml": "writes CLAUDE.md across the fleet; contents:write is real here", }