fix(748): declare permissions: on all six workflows, and prove the declaration binds #856

Closed

timothy wants to merge 12 commits from fix/748-workflow-permissions-v2 into main

Author	SHA1	Message	Date
timothy	baf8e8083d	docs(748): the phantom key survived one directory outside the sweep PR Gates / CI image pin matches docker/ci (pull_request) Failing after 12s Details Build ErsatzTV Image / CI toolchain image resolves (pull_request) Successful in 12s Details PR Gates / Docs update reminder (pull_request) Successful in 17s Details Build ErsatzTV Image / Delimiter ban (release path) (pull_request) Successful in 22s Details PR Gates / Fix proofs (Proves trailers) (pull_request) Successful in 13s Details PR Gates / decisions lifecycle (pull_request) Successful in 21s Details review-verdict/h10 Awaiting review verdict for baf8e80 Details Review verdict / Set review-verdict status (pull_request_target) Successful in 17s Details PR Gates / Script lint and tests (ruff + pytest) (pull_request) Successful in 6m14s Details Build CI Toolchain Image / Build & push CI image (push) Successful in 23s Details Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 8m58s Details Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 6m18s Details Build ErsatzTV Image / Build & push image (amd64) (pull_request) Skipped Details Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 6m10s Details Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 8s Details Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 5s Details Round 7 swept `docs/` for citations of `land-toolchain-image-change-separately` — which is a memory filename, not a decision key — and fixed all three. The fourth was in the test docstring the SAME commit restructured: the sweep population was a directory rather than "every place the key is cited". Verified tree-wide this time (`grep -r` excluding .git): zero citations remain. Decisions-Edit: yes Refs: #748	2026-08-27 22:56:00 +02:00
timothy	05dbadbd03	fix(748): round 7 — and my self-deleting guard was itself dead code Review findings on the ci-image drop, plus one I found only because I mutated the claim instead of reading it. THE ONE THAT MATTERED. Round 6 added a "self-deleting" assert so the ci-image exemption cannot outlive #744 — and it was UNREACHABLE in exactly the case it exists for. The loop skipped any workflow that declares a block before it reached the EXEMPT branch, so giving ci-image.yml a block left the test GREEN. `a-guard-that-never-executed-proves-nothing`, in a guard written to prevent a silent hole, and I had already written "self-deleting" in a commit message. Caught by running the mutation and verifying it applied, not by reading the code. EXEMPT is now tested BEFORE `declares`, with a comment saying why that order is load-bearing. Both directions re-proved: ci-image gains a block -> red; renovate loses its block -> red; restored -> green. `docs/ci-cd.md` asserted P and not-P about the same file in one sentence — I had APPENDED an "EXCEPT ci-image.yml" to the round-2 claim that ci-image's persisted credential is read-only by declaration, rather than deleting the claim. Both halves were standing, in the section a reader consults for precisely that fact. Round-2 clause deleted. `ci.gate-trigger-base-resolved` still summarised, two sentences after the carve-out that falsifies it, that "the route is closed for the workflows that exist TODAY" — body AND `rule:`, so in the catalog. ci-image.yml IS a current instance and IS open. Now: NARROWED, not closed, with both reasons. `ci.actions-credential-scoping` still said five hold `code: read` — four do; five DECLARE, of which one declares write. Body and `rule:`. The same fact was four in ci-cd.md and five in the record, which is the divergence pattern from round 3. `land-toolchain-image-change-separately` is NOT a decision key — no record, no archive entry, not in the catalog. It is a memory filename, and round 6 propagated it into two more places including the catalog. That is the breadcrumb defect this repo has a rule against. All citations now point at `docs/ci-cd.md` -> "CI toolchain image", including the one that predates this PR. Strength: "any branch push runs its head-resolved YAML" overstated the trigger — only a push touching `docker/ci/**` or the file itself fires it. Kept the threat claim, which holds because a forging edit necessarily touches the file. Decisions-Edit: yes Refs: #748 #744 #835	2026-08-27 22:35:46 +02:00
timothy	5fe4af5708	fix(748): drop ci-image.yml — it rides with #744 , and CI proved why `ci-image-pin` (a BLOCKING job) went red on the PR head, exactly as `ci.actions-credential-scoping` predicted: its `expected` is `git log -1 -- docker/ci .gitea/workflows/ci-image.yml`, so a PR that EDITS `ci-image.yml` re-points `expected` at its own commit and the existing pin reads as stale. I had read that warning and bundled the file anyway. Two further reasons not to pay the toll instead: * clearing it means letting ci-image.yml publish a toolchain image from this branch, then bumping all five pins — and a rebase recharges it. That is why `land-toolchain-image-change-separately` exists. * `fix/744-ci-image-branch-filter` is ACTIVE in another session right now (queued workflow_dispatch run 2340), and #744 exists precisely to change this file alone. Editing it here would collide. Verified my push did NOT publish an image: no ci-image run from this branch. So `ci-image.yml` is reverted to `origin/main` and the guard carries a ONE-NAME literal exemption with the reason and the tracking issue — the same exemption #746 already carries for that file's `persist-credentials`, and ersatztv#835 already tracks deleting BOTH once #744 lands. Mutation-checked that the exemption does not blunt the guard: dropping `renovate.yml`'s block still reds. THE CLAIM THAT HAD TO CHANGE, not just the counts. I had written in `ci.gate-trigger-base-resolved` that "no ref-resolved workflow that exists TODAY can post a status". With ci-image.yml excluded that is FALSE, and falsest in the worst place: it declares nothing, so it keeps the owner default, AND its unfiltered `push:` is path-scoped to itself, so any branch push runs its head-resolved YAML with a status-capable token. Both the record's `rule:` and its body now say ci-image.yml is the exception and is the worst-placed of the six, with #744 named as the issue that closes the route — scoping would not have closed it anyway, since scoping bounds what a job may DO and only a filter bounds whether it RUNS. Swept every dependent count and claim rather than the ones I remembered: six->five declaring, five->four holding `code: read`, and the reverted assertion that ci-image.yml's persisted checkout credential is read-only by declaration — it is not; it is still write-capable on the owner default, in `docs/ci-cd.md`, `ci.actions-credential-scoping` (body AND `rule:`), `release.verdict-status-check` and `release.main-direct-push-disabled`. New "carve-out" subsection in docs/ci-cd.md states it as a scheduling constraint, explicitly not a safety judgement. Decisions-Edit: yes Refs: #748 #744 #835	2026-08-27 22:16:55 +02:00
timothy	548e058ea5	docs(748): align the persist-credentials clause with its own record body Build ErsatzTV Image / CI toolchain image resolves (pull_request) Successful in 7s Details Build ErsatzTV Image / Delimiter ban (release path) (pull_request) Successful in 24s Details PR Gates / CI image pin matches docker/ci (pull_request) Failing after 10s Details PR Gates / Docs update reminder (pull_request) Successful in 20s Details PR Gates / Fix proofs (Proves trailers) (pull_request) Successful in 11s Details PR Gates / decisions lifecycle (pull_request) Successful in 24s Details Review verdict / Set review-verdict status (pull_request_target) Successful in 9s Details review-verdict/h10 Review-verdict: MERGEABLE @ 548e058 (base: main) Details PR Gates / Script lint and tests (ruff + pytest) (pull_request) Successful in 5m46s Details Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 9m4s Details Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 6m11s Details Build ErsatzTV Image / Build & push image (amd64) (pull_request) Skipped Details Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 6m1s Details Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 9s Details Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 6s Details The last item from the review, taken rather than deferred: it was graded non-blocking (generically true, and wrong in the CONSERVATIVE direction — it overstates a credential's capability, so nobody is told something is safe that isn't), but it is the same body-vs-`rule:` split as rounds 4 and 5, in the record this PR is centred on, and it is one clause in a file already open. `ci.actions-credential-scoping`'s `rule:` said `actions/checkout` "persists a write-capable credential"; its body two paragraphs down, corrected in round 2, says that since #748 the one persisted credential is read-only by declaration. The rule now says the same, and the catalog carries it. Decisions-Edit: yes Refs: #748	2026-08-27 21:44:57 +02:00
timothy	8d071d04ff	fix(748): the same field's TAIL still said to wait for a Gitea upgrade Round 5, and the last mirror the concept-sweep finds. Round 4 corrected the middle of `ci.gate-trigger-base-resolved`'s `rule:` and left its tail, so one YAML line said both "five hold `code: read` since #748, under today's permissive default" and, ~600 characters later, "the injected token stays write-capable until Gitea >=1.26 with a Restricted default". Doubly stale. The version half was already wrong independently of #748: the instance went 1.25.4 -> 1.27.1 on 2026-08-05, so ">=1.26" has been satisfied for three weeks, and `ci.actions-credential-scoping` has said so since that date. A reader of that tail was told to wait for an upgrade that had already happened. Replaced with the framing the rest of the corpus now uses: the Restricted default is a separate OPEN lever whose effect on a declared `code: write` is unmeasured, and it is not something to wait on a Gitea release for. Catalog regenerated; verified the stale clause is absent from both the record and `docs/decisions/README.md`, and the corrected one present in both. Lesson this PR keeps re-teaching, recorded because it cost five rounds: a "sweep" that greps the wording I just corrected finds only what I already fixed. Rounds 3, 4 and 5 each found the next instance one structural level out — body prose, then the `rule:` field the catalog mirrors, then the far end of that same field. Sweep by SUBJECT, and treat a long generated-from field as needing its own read end to end rather than a match count. Decisions-Edit: yes Refs: #748	2026-08-27 21:28:45 +02:00
timothy	01e51830ed	fix(748): the boundary was one level up — the `rule:` field, not the body Round 4. Round 3 corrected the BODY of `ci.gate-trigger-base-resolved` and left its `rule:` frontmatter, which `build_decisions_catalog.py` mirrors verbatim into `docs/decisions/README.md`. Three compounding consequences, all now fixed: * it contradicted the body of its own record twelve lines below it; * the catalog is where CLAUDE.md says a decision lookup STARTS, so the stale version was the one a future editor read FIRST and the corrected body sat behind it; * as stated it was false for this repo today, not merely dated — "any ref-resolved workflow can still forge `review-verdict/h10`". This is the same fixed-the-site-not-the-class shape recurring one level up. My round-3 sweep grepped body prose and missed it because the `rule:` field is a single long YAML line and my pattern was anchored on wording I had already corrected. Re-swept by subject with a wider pattern across `docs/` and `CLAUDE.md`; the remaining hits are past-tense, already corrected, or about a different subject (the secret store; "credentials that CAN post a status", which stays true). The rule now says what #748 does and does not close: current instances closed, the SHAPE open, and the new guard is what covers a newly added workflow. GRADING, not just correcting. The body asserted "none of them can POST a status" at full strength while `ci-cd.md` and `ci.actions-credential-scoping` both grade the same claim as resting on the TOP-LEVEL declaration form, which is inferred rather than probed. The corpus graded it in two places and asserted it ungraded in the third — the record about forgery routes. Both the rule and the body now carry the qualifier. Also: `docker-build.yml`'s round-3 meta-note had been inserted between "uses the REGISTRY_* PAT" and "needs no `packages:` unit for that reason", stealing the antecedent of "that reason". Moved to the end of the block. And `ci-cd.md` understated `ci-image.yml`'s trigger filter (`docker/ci/**` AND its own file, not just its own file). Decisions-Edit: yes Refs: #748	2026-08-27 21:15:05 +02:00
timothy	efe711c5a9	fix(748): round 3 — cite jobs not lines, and fix the boundary not the site Every finding from the re-review of the round-2 fix. The round-2 comment fix shipped four line numbers that its own two added lines invalidated (:186/:456/:617 and :144 were already 188/458/619 and 146 in the committed file) — "broke a number while fixing numbers", in the commit that was fixing a number. Root cause: I transcribed them from the review report, which described the PRE-edit file, instead of re-deriving from the post-edit tree. The fix is not better numbers: docker-build.yml is ~1150 lines and any edit above a citation silently invalidates it, so the sites are now named by JOB (`toolchain-preflight`; `test`/`migrations`/`functional-e2e`), which the YAML itself defines and an edit cannot shift. Comment says so, so the next editor does not reintroduce line numbers. Round 2 split the per-unit warrants in the record but left `docs/ci-cd.md` asserting the retracted one — one wrong copy became two divergent copies. Both now state it the same way and per unit: the fence shows `issues: read`; what shows `pull-requests: read` is the `=success`, reachable only via `complete=yes`. The ⚠️ was over-generalized in round 1 and over-reassuring in round 2: it said "the other five are ordinary" and then justified two. Split — docker-build and pr-checks are `on: pull_request` and ARE exercised by an editing PR; ci-image (`push`, self-path-filtered), dependency-scan and renovate (`schedule` / `workflow_dispatch`) are NOT, and their first run is post-merge or next schedule. `ci-image.yml`'s comment claimed the other five "persist no credential (`persist-credentials: false`)". `renovate.yml` sets no such key — it persists nothing because it never checks out. Four of five set the flag. THE BOUNDARY, not the site. Round 2 fixed the one stale-claim site the review named. A grep by SUBJECT rather than by the corrected wording found the same superseded inventory in two more records, both of them ones a future editor resolves from the catalog when reasoning about forgery routes: * `ci.gate-trigger-base-resolved` — "Gitea injects GITEA_TOKEN into EVERY job, defaulting to read/write, so push- and workflow_dispatch-triggered workflows alike are routes". Now scoped to the workflows that exist today, with the note that the SHAPE is still open and that is what the new guard covers. * `release.verdict-status-check` — the same inventory in the H10 gate's own record. * `release.main-direct-push-disabled` — dated parenthetical, conclusion unaffected, corrected in passing. Guards: `test_..._declares_exactly_the_units_...` now states what it does NOT catch — it TRANSCRIBES the unit set rather than deriving it from the job's API calls, so a new `gh` call needing an undeclared unit leaves it green. Same honesty the sibling expression test already carries. And `test_every_tracked_workflow_declares_a_permissions_block` gained an anti-vacuity assert: `tracked_children` matches direct children only, so a moved `.gitea/workflows/` gave an EMPTY population and the test passed having examined nothing — the #778 shape its own docstring invokes. Measured before the guard (`git mv .gitea/workflows .gitea/wf-renamed` -> 1 passed); measured after -> red, and green again on restore. Correcting round 2's commit message: it said both new tests run over the git-derived `_workflow_files()` population. Only the second does; the first addresses `review-verdict.yml` directly. Also scoped the record's new measurement heading — it re-probes ONE item of the 1.25.4 caveat (the `permissions:` question) and does not discharge the scope enum, the `reqRepoWriter` gate or the `write:package` 403, which stay dated. Decisions-Edit: yes Refs: #748	2026-08-27 20:59:23 +02:00
timothy	f4c44bfd6a	fix(748): address the cold review — stale claim, probe scope, and a guard Round 2, all five findings plus the guard gap. HIGH — `docs/ci-cd.md` (review-verdict gate section) still asserted "Gitea injects a write-capable GITEA_TOKEN into every job" and that only a Restricted flip binds it. Both are false for this repo after this change, and that copy sits in the section someone reads WHILE editing the gate — i.e. the copy most likely to be believed, arguing that the five new `code: read` blocks are decorative. Rewritten to point at "Workflow token scope". MEDIUM — "the five read-only declarations bind today" was inferred from a JOB-LEVEL probe while the five use TOP-LEVEL. Both docs now state the level the probe carried (job-level, on `set-verdict-status`, the shipped shape), and mark the top-level form as inferred — noting it fails safe and that docker-build/pr-checks are `on: pull_request`, so this PR's own CI exercises it. MEDIUM — the new 1.27.1 measurement had been inserted ABOVE the paragraph saying "everything above is pinned to 1.25.4", which swept it into a caveat it contradicts. Moved below, and labelled as the re-probe that caveat asks for. MEDIUM — the record claimed the fence and the start marker showed both read units were exercised. The fence shows `issues: read`; what shows `pull-requests: read` is the `=success`, which is reachable only via `complete=yes`. Warrants now stated per unit. MEDIUM — `docker-build.yml`'s comment said "one commit-status GET" (three, at :186/:456/:617) and omitted `ETV_REGISTRY_AUTH` at :144 entirely. Enumeration corrected; the conclusion it supports was already right. LOW — `ci-image.yml`'s comment said read-only-persisted was "a gain this file gets and the others already had". The others persist NO credential; the two are not interchangeable and this file has the weaker property. GUARD — nothing pinned `code: write` on the gate job, and by that workflow's own base-resolved property the editing PR can never catch its removal. Two tests in `test_pr_changed_files.py`, both over the git-derived `_workflow_files()` population: the gate job's units by EQUALITY (an extra unit is a finding too — this is the repo's only CI repo-write), and every tracked workflow declaring a block at some level. Mutation-proved, all four reddening the intended test and the tree restoring green: code:write->read; drop pull-requests:read; add a top-level block to review-verdict.yml; drop dependency-scan.yml's block. Decisions-Edit: yes Refs: #748	2026-08-27 20:36:45 +02:00
timothy	84f001eec4	fix(748): docker-build has three commit-status GET steps, not one Self-review of the comment I wrote: `ETV_STATUS_AUTH` is set on three steps (:186, :456, :617), each invoking scripts/ci-detect-already-validated.sh, not one. The claim about WHICH credential they use was right; the count was not. Refs: #748	2026-08-27 20:28:00 +02:00
timothy	f39137d7ce	docs(748): the probe cannot clear the Restricted default — say so Self-review correction. Both controls ran while the owner-level Actions default was `permissive`, so the probe shows the declaration binds DOWNWARD (a declared `code: read` really loses write). It says nothing about what a declared `code: write` receives under a RESTRICTED default. That distinction is the whole risk. GitHub's semantics let `permissions:` only narrow, never widen past the default; if Gitea copies that, Restricted caps the gate job at read and `review-verdict/h10` becomes unwritable — with no force-merge escape since #743. The previous wording said the flip "is now safe to apply here", which the evidence does not support. Corrected in both `docs/ci-cd.md` and the record's `rule` + surviving-routes prose: the flip stays gated on re-running the scratch-base probe UNDER Restricted, which is why server-management#714 stays open. Decisions-Edit: yes Refs: #748	2026-08-27 20:17:14 +02:00
timothy	ca342db70b	docs(748): record that `permissions:` binds, measured by matched probe Adds `docs/ci-cd.md` -> "Workflow token scope" (the section the inline comments on all six workflows point at) and corrects `ci.actions-credential-scoping`, which had carried "whether `permissions:` is honored here is still UNPROBED" and an explicit "do NOT add a `permissions:` key" since 2026-08-05. Settled by a matched scratch-base probe pair on Gitea 1.27.1, differing only in the gate job's single permission unit: code: write -> job success, `Posted review-verdict/h10-PROBE=success`, status creator: null code: read -> job failure at the POST, curl exit 22 (HTTP >=400 under -f), NO status written Both reached the classify step's start marker and both logged the retarget fence as trusted, so the read units were exercised and neither run was the #751 dropped-step shape. Both ran while the owner default was still `permissive`, which is what establishes the declaration binds irrespective of that default. The negative control is the load-bearing half: a green positive alone is equally consistent with `permissions:` being a no-op sitting beside a permissive default. Also corrects the two places that framed the persisted checkout credential's write-capability as decided by the instance default; since this change it is decided by `ci-image.yml`'s own `code: read` declaration. Decisions-Edit: yes Refs: #748	2026-08-27 20:06:29 +02:00
timothy	81767af084	fix(748): declare explicit token permissions on all six workflows So the owner-level Actions default can move to Restricted (server-management#714) without this repo's merge gate losing the ability to write `review-verdict/h10`. review-verdict.yml (job-level) code: write, issues: read, pull-requests: read the other five (top-level) code: read `code: write` because Gitea has no `statuses` scope — status-write IS Code-write (`reqRepoWriter(unit.TypeCode)`), per `ci.actions-credential-scoping`. The two read units are NOT spare: the gate job also GETs `/issues/{n}/timeline` (the #706 retarget fence) and, via `scripts/pr-changed-files.sh`, `/pulls/{n}` and `/pulls/{n}/files` (the #698 exemption path). Enumerated against every authenticated call the job makes; the set covers it exactly, with nothing over. Corrects #748's body: `docker-build.yml` does NOT write a status with the injected token. It has no `GITEA_TOKEN` reference at all — its `ETV_STATUS_AUTH` is the scoped REGISTRY_* PAT doing a read-only GET. Only review-verdict.yml needs write. No `packages:` unit is needed anywhere: all five `container:` blocks carry explicit `credentials:` using the REGISTRY_* PAT, so no job pulls the toolchain image with the injected token. refs #748	2026-08-27 19:58:11 +02:00