diff --git a/.gitea/workflows/ci-image.yml b/.gitea/workflows/ci-image.yml index faf1e848d..3b04cae4b 100644 --- a/.gitea/workflows/ci-image.yml +++ b/.gitea/workflows/ci-image.yml @@ -84,6 +84,21 @@ env: REGISTRY: 192.168.1.95:3000 CI_IMAGE: 192.168.1.95:3000/timothy/ersatztv-ci +# Explicit token scope (ersatztv#748) so the owner-level Actions default can move to Restricted +# (server-management#714). Declaring `permissions:` is EXHAUSTIVE, not additive: a unit omitted here +# is NOT granted, and that holds at any owner default — it is not conditional on Restricted being on. +# Only `review-verdict.yml` needs write; it declares that at the job and says why there. Full +# rationale and the per-workflow credential audit: docs/ci-cd.md -> "Workflow token scope". +# This workflow's registry pushes authenticate with the scoped REGISTRY_* PAT +# (`ci.actions-credential-scoping`), so the injected GITEA_TOKEN serves only its single +# `actions/checkout`. This file was the one workflow #748 could not originally reach: editing it +# re-pointed `ci-image-pin`'s `expected` at the editing commit and reddened a BLOCKING job, and its +# own `paths:` made the edit publish an image. ersatztv#744 took this path out of both +# (`ci.toolchain-image-publish-is-a-dispatch`), so the exemption that briefly existed here is DELETED +# rather than documented — which is what ersatztv#835 asked for. +permissions: + code: read + jobs: build: name: Build & push CI image diff --git a/.gitea/workflows/dependency-scan.yml b/.gitea/workflows/dependency-scan.yml index 0245c981d..80707bb4c 100644 --- a/.gitea/workflows/dependency-scan.yml +++ b/.gitea/workflows/dependency-scan.yml @@ -33,6 +33,16 @@ env: DOTNET_CLI_USE_MSBUILD_SERVER: "0" MSBUILDDISABLENODEREUSE: "1" +# Explicit token scope (ersatztv#748) so the owner-level Actions default can move to Restricted +# (server-management#714). Declaring `permissions:` is EXHAUSTIVE, not additive: a unit omitted here +# is NOT granted, and that holds at any owner default — it is not conditional on Restricted being on. +# Only `review-verdict.yml` needs write; it declares that at the job and says why there. Full +# rationale and the per-workflow credential audit: docs/ci-cd.md -> "Workflow token scope". +# Holds no registry credential and reads nothing from the Gitea API; the injected GITEA_TOKEN serves +# only its one `actions/checkout`. +permissions: + code: read + jobs: scan: name: NuGet vulnerable packages diff --git a/.gitea/workflows/docker-build.yml b/.gitea/workflows/docker-build.yml index b34f61097..7e2d2971f 100644 --- a/.gitea/workflows/docker-build.yml +++ b/.gitea/workflows/docker-build.yml @@ -113,6 +113,24 @@ env: DOTNET_CLI_USE_MSBUILD_SERVER: "0" # no persistent MSBuild server process MSBUILDDISABLENODEREUSE: "1" # MSBuild worker nodes exit with the build instead of lingering +# Explicit token scope (ersatztv#748) so the owner-level Actions default can move to Restricted +# (server-management#714). Declaring `permissions:` is EXHAUSTIVE, not additive: a unit omitted here +# is NOT granted, and that holds at any owner default — it is not conditional on Restricted being on. +# Only `review-verdict.yml` needs write; it declares that at the job and says why there. Full +# rationale and the per-workflow credential audit: docs/ci-cd.md -> "Workflow token scope". +# Every credentialed thing this file does uses the scoped REGISTRY_* PAT, never the injected token: +# its registry pushes, its five `container:` image pulls, its three commit-status GET steps +# (`ETV_STATUS_AUTH` in jobs `test`, `migrations` and `functional-e2e`, each a read-only GET via +# scripts/ci-detect-already-validated.sh) and its registry tag READ (`ETV_REGISTRY_AUTH` in job +# `toolchain-preflight`, via scripts/ci-toolchain-image-resolves.sh). The injected token therefore +# serves only its eight `actions/checkout` steps. Note this file needs no `packages:` unit for that +# same reason: the `container:` blocks carry explicit `credentials:`. +# (Sites above are named by JOB, not by line number: this file is ~1150 lines, so any edit above a +# citation silently invalidates it — which is how the first version of this comment went stale two +# lines after it was written.) +permissions: + code: read + jobs: # Answers "is the toolchain image still there?" in ONE place, so a deleted pin does not read as # five broken jobs and a broken diff (ersatztv#772). Deliberately container-free and deliberately diff --git a/.gitea/workflows/pr-checks.yml b/.gitea/workflows/pr-checks.yml index 29d00eff1..a336189aa 100644 --- a/.gitea/workflows/pr-checks.yml +++ b/.gitea/workflows/pr-checks.yml @@ -42,6 +42,16 @@ concurrency: group: ersatztv-pr-gates-${{ github.ref }} cancel-in-progress: true +# Explicit token scope (ersatztv#748) so the owner-level Actions default can move to Restricted +# (server-management#714). Declaring `permissions:` is EXHAUSTIVE, not additive: a unit omitted here +# is NOT granted, and that holds at any owner default — it is not conditional on Restricted being on. +# Only `review-verdict.yml` needs write; it declares that at the job and says why there. Full +# rationale and the per-workflow credential audit: docs/ci-cd.md -> "Workflow token scope". +# Holds no secrets at all and reads nothing from the Gitea API; the injected GITEA_TOKEN serves only +# its five `actions/checkout` steps. +permissions: + code: read + jobs: # BLOCKING (ersatztv#390): the CI toolchain image pin in docker-build.yml must name the short sha of # the last commit to touch the image's SOURCES (`docker/ci/**`). Read that as "the image ci-image.yml diff --git a/.gitea/workflows/renovate.yml b/.gitea/workflows/renovate.yml index 114eb262e..e1b21623b 100644 --- a/.gitea/workflows/renovate.yml +++ b/.gitea/workflows/renovate.yml @@ -45,6 +45,18 @@ concurrency: group: ersatztv-renovate cancel-in-progress: false +# Explicit token scope (ersatztv#748) so the owner-level Actions default can move to Restricted +# (server-management#714). Declaring `permissions:` is EXHAUSTIVE, not additive: a unit omitted here +# is NOT granted, and that holds at any owner default — it is not conditional on Restricted being on. +# Only `review-verdict.yml` needs write; it declares that at the job and says why there. Full +# rationale and the per-workflow credential audit: docs/ci-cd.md -> "Workflow token scope". +# This workflow has no checkout step and never uses the injected GITEA_TOKEN for anything. Renovate's +# own branch/PR writes go through RENOVATE_TOKEN, a dedicated bot PAT the Actions default does not +# govern, and its container image comes from Docker Hub. Read-only is declared to STATE that the +# injected token is unused, not because any step needs it. +permissions: + code: read + jobs: renovate: name: Renovate diff --git a/.gitea/workflows/review-verdict.yml b/.gitea/workflows/review-verdict.yml index 024ffb82b..00fa3f2d8 100644 --- a/.gitea/workflows/review-verdict.yml +++ b/.gitea/workflows/review-verdict.yml @@ -118,6 +118,25 @@ jobs: set-verdict-status: name: Set review-verdict status runs-on: small # a few API calls; keep it off the build runners + # Explicit token scope (ersatztv#748), so the owner-level Actions default can be set to + # Restricted (server-management#714) without this job losing the ability to write the gate. + # + # DECLARING THIS IS EXHAUSTIVE — it is not additive. From the moment it merges, this job holds + # EXACTLY these units even while the owner default is still `permissive`. A missing unit does not + # degrade gracefully: the `curl -sf` dies under `set -euo pipefail` and `review-verdict/h10` is + # never posted, which blocks EVERY merge to `main` — including the PR that would repair it, and + # `block_admin_merge_override: true` (ersatztv#743) means there is no force-merge escape. Change + # this block only with the scratch-base-branch probe in `docs/ci-cd.md`; this workflow is never + # exercised by its own PR. + # + # Why `code: write` and not something narrower: Gitea has NO `statuses` scope (GitHub-only, and + # explicitly unsupported). `POST /repos/{o}/{r}/statuses/{sha}` is gated by + # `reqRepoWriter(unit.TypeCode)`, so status-write IS Code-write — see `ci.actions-credential-scoping`. + # `contents: write` would also work but additionally grants `releases: write`, which is not used. + permissions: + code: write # POST /statuses/{sha} (the gate) + the /commits/{sha}/status reads + issues: read # GET /issues/{n}/timeline — the retarget-count fence (#706) + pull-requests: read # scripts/pr-changed-files.sh -> GET /pulls/{n}/files (#698 exemption path) steps: # BASE, not head — see the header. `fetch-depth: 1` is enough: nothing here reads history, # only the working tree's `scripts/`. diff --git a/docs/ci-cd.md b/docs/ci-cd.md index afdbf921d..472e975bd 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -147,6 +147,109 @@ A backup, fetch, or migration-smoke failure aborts before the live container is **Also avoid firing several pushes back-to-back** (e.g. a `[skip ci]` commit, then `main`, then a tag, all within ~1s). Observed once on this Gitea instance: the later events were silently dropped — no `ActionRun` records created at all, even though the runner was online and the workflow `active`. Pushing again, spaced out, created the runs normally. If a push/tag doesn't produce a run, re-push (or push an empty commit) rather than assuming the runner is broken. +## Workflow token scope (`permissions:`, ersatztv#748) + +Every workflow in this repo declares `permissions:`. Five declare `code: read` at the top level; +`review-verdict.yml` declares `code: write` + `issues: read` + `pull-requests: read` on its +`set-verdict-status` job and nothing at the top level. That is the whole convention — a new workflow +should declare the narrowest set it needs, and say at the declaration why it needs write if it does. + +**Declaring `permissions:` is exhaustive, not additive.** A unit omitted is a unit not granted, and +that holds *at any owner-level default* — it is not conditional on the instance default being +Restricted. This is the property that makes the block load-bearing rather than decorative, and it is +also what makes a wrong one dangerous: the gate job's API calls are `curl -sf` under +`set -euo pipefail`, so a missing unit is a 403 that kills the step, and `review-verdict/h10` is +never posted. That blocks **every** merge to `main`, including the PR that would repair it, and +since ersatztv#743 (`block_admin_merge_override: true`) there is no force-merge escape. + +`code: write` and not something narrower because **Gitea has no `statuses` scope**. +`POST /repos/{o}/{r}/statuses/{sha}` is gated by `reqRepoWriter(unit.TypeCode)`, so status-write *is* +Code-write (`ci.actions-credential-scoping`). `contents: write` would also work but additionally +grants `releases: write`, which nothing here uses. The two read units are not spare: the job GETs +`/issues/{n}/timeline` (the ersatztv#706 retarget fence) and, through +`scripts/pr-changed-files.sh`, `/pulls/{n}` and `/pulls/{n}/files` (the ersatztv#698 exemption path). + +No workflow needs a `packages:` unit. All five `container:` blocks in `docker-build.yml` carry +explicit `credentials:` using the scoped `REGISTRY_*` PAT, so no job pulls the toolchain image with +the injected token. + +### The binding was measured, not assumed (2026-08-27, Gitea 1.27.1) + +`ci.actions-credential-scoping` recorded for three weeks that whether `permissions:` is honored on +this instance was **unprobed** — 1.26.0 was the version it was said to start binding at, from +release notes rather than from an experiment. It was settled with the scratch-base probe below, run +as a matched pair that differed **only** in one unit: + +| Control | Gate job unit | Result | +|---|---|---| +| Positive | `code: write` | job `success`; `Posted review-verdict/h10-PROBE=success`; `creator: null` | +| Negative | `code: read` | job `failure`; `curl` exit 22 (HTTP ≥400 under `-f`) at the POST; **no status written** | + +Both runs reached the classify step's start marker, so neither failure was the ersatztv#751 +dropped-step shape. What shows each read unit was exercised is a different observation for each, and +they are not interchangeable: + +- **`issues: read`** — both runs logged `Retarget fence: 0 retarget event(s) … (trusted=yes)`, which + requires `count_retargets` to have walked `/issues/{n}/timeline`. +- **`pull-requests: read`** — the positive control posted `=success`, and a `success` is reachable + only through `complete=yes`, which requires `scripts/pr-changed-files.sh` to have paged + `/pulls/{n}` and `/pulls/{n}/files` to a validated end. The fence and the start marker do **not** + show this; the `success` does. + +The negative control is the load-bearing half: it is what shows the declaration *binds* rather than +being a no-op that happens to sit next to a permissive default. Both controls carried the block **at +job level**, on `set-verdict-status` — the same level and the same job as the shipped +`review-verdict.yml`, so the gate's own shape is what was measured. + +Both ran while the owner default was still `permissive`, so the binding holds **today**, without any +flip. One scope limit, stated rather than glossed: the other five workflows declare `permissions:` at +**top level**, a form the probe did not exercise. That is inferred, not measured — but the failure +direction is safe (a top-level key Gitea ignored would be a security no-op, never a brick), and +`docker-build.yml` and `pr-checks.yml` are `on: pull_request`, so this change's own CI exercises +theirs. + +**What the probe does NOT establish — read this before flipping the owner default.** It says nothing +about what a declared `code: write` receives under a **Restricted** default. GitHub's semantics let +`permissions:` only *narrow*, never widen past the default; if Gitea copies that, Restricted would +cap the gate job at read and `review-verdict/h10` would become unwritable — precisely the failure +this section warns about, with no force-merge escape. Both controls ran under `permissive`, so that +case is untested. server-management#714 stays open for exactly this: re-run the scratch-base probe +**under Restricted** before trusting the flip. + +`creator: null` on the positive control matters separately: it is the provenance signal +`review-verdict.yml` relies on to tell a machine-written status from a reviewer's +(`ci.exemption-provenance`, ersatztv#742). Approach A — scoping the built-in token — preserves it. +Moving status writes onto a PAT (approach B) would not, which is why B stays blocked. + +**What this does and does not close.** Scoping bounds what a job may *do*; it never bounds whether +attacker YAML *runs* — that needs a trigger filter (ersatztv#744). And `review-verdict.yml` still +holds repo-write. The property gained is not "workflows can no longer assert the verdict"; it is +that *only a workflow visibly opting into repo-write can*, instead of every job on the instance +holding write implicitly. Real, and weaker than closed. + +⚠️ **Changing `review-verdict.yml`'s block requires the scratch-base probe**, not CI-green on the +editing PR — that workflow is base-resolved and is never exercised by its own PR, so its own checks +say nothing about the edit. See the probe recipe in the review-verdict gate section below. + +The other five split two ways, and only one half is covered by an editing PR's own CI: + +- `docker-build.yml` and `pr-checks.yml` run on `pull_request`, so an editing PR exercises their + blocks directly. (`docker-build.yml`'s `build` job carries + `if: github.event_name != 'pull_request'`, so its registry-push path is first exercised on the + post-merge push to `main` — that reddens `main`, not the gate, and is recoverable.) +- `ci-image.yml` (`push` to `main` only since ersatztv#744, plus `schedule`/`workflow_dispatch`), + `dependency-scan.yml` and `renovate.yml` are **not** exercised by an editing PR at all — their + first run is after merge or on the next schedule. None is a required check, so a wrong block there reddens a non-blocking run rather than the gate, but do not read an + editing PR's green as covering them. + +**`ci-image.yml` is included, and only just.** It was the one workflow this change could not +originally reach: editing it re-pointed `ci-image-pin`'s `expected` at the editing commit and +reddened a **blocking** job, and its own `paths:` made the edit publish a toolchain image. ersatztv#748 briefly +carried a documented one-name exemption for it. ersatztv#744 (`ci.toolchain-image-publish-is-a-dispatch`) +took that path out of both, so the exemption was **deleted rather than documented** — which is the +form ersatztv#835 asked for, and it is why `test_pr_changed_files.py` asserts over the whole derived +population with nothing carved out. + ## Checkouts drop the persisted credential (ersatztv#746) **Every** `actions/checkout` in this repo sets `persist-credentials: false` — with no exemption, @@ -156,9 +259,11 @@ and ersatztv#835 now holds the rule mechanically rather than by convention: index and asserts the flag on every checkout step it finds. Without it the action leaves an `Authorization` header in `.git/config`, and a head-resolved job then hands that credential to every script it runs, not just -to the checkout. That header is write-capable: this instance's Actions default token permission is -`permissive`, where it stands after a Restricted trial was reverted on 2026-08-05, and flipping it -back is ersatztv#748. No job that checks out +to the checkout. That header is write-capable *only while the job's token is*: this instance's Actions default +token permission is `permissive`, where it stands after a Restricted trial was reverted on +2026-08-05 — but since ersatztv#748 that default no longer decides it. Every workflow declares +`permissions:` (see the section above), so every persisted header in this repo is read-only by +declaration. Re-applying the owner-level default is server-management#714. No job that checks out loses anything: none of them push with git, and anonymous access covers the fetches because the repo is public — `private: false`, and `GET /info/refs?service=git-upload-pack` answers unauthenticated, measured 2026-08-26 from the LAN. That measures the repository, not a runner's network path to it, diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 057c85e72..b4021c709 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -38,7 +38,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `blazor.rollback-tag` | The commit immediately preceding the Blazor-removal merge is tagged `blazor-final` (not a `v*` tag, so it doesn't trigger a prod release build) as the documented rollback/restore path. | 2026-07-11 | [link](records/blazor/rollback-tag.md) | | `blazor.ui-removed` | The legacy Blazor Server UI (`Pages/`, `Shared/`, `ViewModels/`, `Validators/`, MudBlazor + 8 other packages, Blazor Startup wiring) is fully deleted now that the SPA has parity; the legacy `MapWhen` branch is kept only for controllers/docs/OpenAPI/`LegacyUiRedirects`, and the catch-all fallback 302s any unmatched non-api/artwork/docs/openapi path to `/app`. | 2026-07-11 | [link](records/blazor/ui-removed.md) | | `channel.origin-marker` | A new `Channel.Origin` (`ChannelOrigin` enum — `Unknown`/`UserCreated`/`AutoTuned`) records how a channel row was created and is stamped exactly once at insert (`AutoTuned` in `CreateChannelFromLineupHandler`, `UserCreated` in `CreateChannelHandler`), and is never mutated on a later edit. It is surfaced as a raw `origin` field on `ChannelResponseModel`; the SPA badges only `AutoTuned`. Rows predating the column read `Unknown` — provenance is **not** back-filled. | 2026-07-23 | [link](records/channel/origin-marker.md) | -| `ci.actions-credential-scoping` | Any credential reachable from an Actions job is scoped to what that job needs. The container-registry secret `REGISTRY_PASSWORD` is a personal access token scoped `write:package` + `read:repository` — never an account PASSWORD. This matters because Gitea has NO `status` token scope: `POST /repos/{o}/{r}/statuses/{sha}` is gated by `reqRepoWriter(unit.TypeCode)`, so ANY credential that can write the repository can forge `review-verdict/h10`, the required context that is supposed to make merge-consent derived rather than assertable. Package-write IS a separate scope, so the registry credential can be made status-incapable at no cost: `scripts/ci-detect-already-validated.sh` only GETs. Do NOT add a `permissions:` key to constrain the injected `GITEA_TOKEN` on the assumption that it binds — below Gitea 1.26.0 it is silently a NO-OP, which is worse than absent because it reads in review as a constraint. That version precondition NO LONGER HOLDS: this instance was upgraded 1.25.4 -> 1.27.1 on 2026-08-05. What has NOT changed is that the consequence is unverified — whether `permissions:` is honored here is still UNPROBED, and there is still no API surface for it (`/api/v1/settings/actions` 404s at 1.27.1). The instance default HAS since been probed and is NOT unknown: it was set to Restricted, verified, and reverted to `permissive` on 2026-08-05 (#748), which is where it stands. Probe before relying on it; do not read the upgrade alone as the constraint now working. Scoping is necessary and not sufficient: it bounds what a job may DO, never whether attacker YAML runs at all, so a self-referencing trigger needs its own filter. That landed for `ci-image.yml` in #744 (`ci.toolchain-image-publish-is-a-dispatch`) — deliberately NOT bundled here, because until it also removed the file from `ci-image-pin`'s `expected`, editing it re-pointed that job at the editing commit and reddened a blocking check. A second, separate consequence of the same boundary: `actions/checkout` persists a write-capable credential into `.git/config` unless `persist-credentials: false` is set, and all 16 of this repo's checkouts now set it — 15 in #746 and `ci-image.yml`'s in #744, once `ci.toolchain-image-publish-is-a-dispatch` removed the two mechanical reasons it was excluded; the convention is held with no exemption list by `scripts/tests/test_workflow_persist_credentials.py` (#835). Ordering is part of the rule: unmask the dependent fetches FIRST, because until then a credential regression presents as an empty changed-file set and a silently skipped check rather than a red job. This record closes ONE route. It does not close the class, and the later sections say exactly what survives — read them before citing this record as a mitigation. | 2026-08-05 | [link](records/ci/actions-credential-scoping.md) | +| `ci.actions-credential-scoping` | Any credential reachable from an Actions job is scoped to what that job needs. The container-registry secret `REGISTRY_PASSWORD` is a personal access token scoped `write:package` + `read:repository` — never an account PASSWORD. This matters because Gitea has NO `status` token scope: `POST /repos/{o}/{r}/statuses/{sha}` is gated by `reqRepoWriter(unit.TypeCode)`, so ANY credential that can write the repository can forge `review-verdict/h10`, the required context that is supposed to make merge-consent derived rather than assertable. Package-write IS a separate scope, so the registry credential can be made status-incapable at no cost: `scripts/ci-detect-already-validated.sh` only GETs. `permissions:` on a workflow/job DOES bind on this instance — MEASURED 2026-08-27 on 1.27.1 by matched scratch-base probe PRs differing only in one unit, the block carried at JOB level on `set-verdict-status` (`code: write` posted the probe status; `code: read` 403ed the POST, curl exit 22, no status written), so all six workflows here now declare it (#748). Two properties that make it usable: the declaration is EXHAUSTIVE, not additive — a unit omitted is not granted — and it binds while the owner-level default is `permissive`, which is what makes the five `code: read` declarations effective TODAY rather than only after a flip. NOT established: what a declared `code: write` does under a RESTRICTED default. GitHub semantics let `permissions:` only narrow, never widen past the default, and if Gitea copies that, Restricted would cap the gate job at read and `review-verdict/h10` would stop being writable — the exact catastrophic case. The probe ran under `permissive` and CANNOT rule that out. Flip the owner default only behind the scratch-base probe re-run under Restricted (server-management#714, still open for this reason). The earlier form of this rule said the opposite ("do NOT add a `permissions:` key on the assumption that it binds — below Gitea 1.26.0 it is silently a NO-OP"); that was correct at 1.25.4 and is retained here so a reader meeting the old advice recognises it as superseded. There is still no API surface for the owner-level default (`/api/v1/settings/actions` 404s at 1.27.1). The instance default HAS since been probed and is NOT unknown: it was set to Restricted, verified, and reverted to `permissive` on 2026-08-05 (#748), which is where it stands. Probe before relying on it; do not read the upgrade alone as the constraint now working. Scoping is necessary and not sufficient: it bounds what a job may DO, never whether attacker YAML runs at all, so a self-referencing trigger needs its own filter. That landed for `ci-image.yml` in #744 (`ci.toolchain-image-publish-is-a-dispatch`) — deliberately NOT bundled here, because until it also removed the file from `ci-image-pin`'s `expected`, editing it re-pointed that job at the editing commit and reddened a blocking check. A second, separate consequence of the same boundary: `actions/checkout` persists whatever the job token can do into `.git/config` unless `persist-credentials: false` is set — read-only everywhere since #748 declared `permissions:` on all six workflows — and all 16 of this repo's checkouts now set it — 15 in #746 and `ci-image.yml`'s in #744, once `ci.toolchain-image-publish-is-a-dispatch` removed the two mechanical reasons it was excluded; the convention is held with no exemption list by `scripts/tests/test_workflow_persist_credentials.py` (#835). Ordering is part of the rule: unmask the dependent fetches FIRST, because until then a credential regression presents as an empty changed-file set and a silently skipped check rather than a red job. This record closes ONE route. It does not close the class, and the later sections say exactly what survives — read them before citing this record as a mitigation. | 2026-08-05 | [link](records/ci/actions-credential-scoping.md) | | `ci.batch-pushes-no-cancel-route` | Hold review fixes, doc corrections and format fixes locally and push **once** — a superseded run cannot be cancelled from the agent side and holds a runner slot until it finishes. | 2026-07-21 | [link](records/ci/batch-pushes-no-cancel-route.md) | | `ci.build-once-rejected` | CI build-once (a shared compile artifact across jobs) was implemented, measured, and rejected for a 40-85% wall-clock regression; keep the #420 cross-run tree-identity skip instead. | 2026-07-18 | [link](records/ci/build-once-rejected.md) | | `ci.cancelled-is-not-a-verdict` | Treat a `cancelled` conclusion as "no verdict" — never as pass or fail — and report FAILED and CANCELLED counts separately in any CI monitor. THE COMBINED COMMIT-STATUS ENDPOINT CANNOT EXPRESS THIS: `GET /repos/{o}/{r}/commits/{sha}/status` has states `success`/`failure`/`pending`/`error` and NO `cancelled`, so it reports a cancelled job as `failure`. Anything polling that endpoint — which is what a CI monitor naturally polls, because it is the per-sha view the merge gate reads — must resolve the job-level `conclusion` via `actions/runs/{id}/jobs` before reporting a red. | 2026-07-21 | [link](records/ci/cancelled-is-not-a-verdict.md) | @@ -49,7 +49,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `ci.exemption-provenance` | The three inputs the exemption decision rests on must each be bound to something the judged PR cannot mutate. (1) BASE — `scripts/pr-changed-files.sh` takes the expected base BRANCH as a REQUIRED 5th argument and re-reads it before and after paging, because `/pulls/{n}/files` diffs against the PR's live base and retargeting moves the answer without moving the head sha; the workflow passes `github.event.pull_request.base.ref` from the `pull_request_target` payload, which a retarget cannot rewrite. (2) BOT EXEMPTION — an author match is necessary but never sufficient: `pull_request.user.login` is the PR's immutable CREATOR while its head is not, so the exemption additionally requires EVERY changed path to be a dependency manifest (`Directory.Packages.props` or `.config/dotnet-tools.json`, and ONLY those — the npm manifests are excluded because `package.json` `scripts` are executed by CI). (3) INHERITED SUCCESS — the never-overwrite short-circuit fires for a `success` only when it is POSITIVELY identified as an APPROVED REVIEWER's verdict for THIS base, meaning a `.creator.login` that is a member of the workflow's `H10_REVIEWERS` allow-list (an existing `failure` takes the weaker ATTRIBUTABLE test and needs no membership, because inheriting a rejection can only withhold an exemption while re-deriving one can turn it green on an exempt PR) — non-null was too weak, since any account's credential satisfies it and `RENOVATE_TOKEN` is one that cannot be scoped down (#742) — AND a `Review-verdict:` description AND, when that description records a base (`(base: …)`, `release.verdict-status-check`), a base matching the PR's — tested by requiring the description to END with the exact literal `(base: )` and to contain exactly ONE such marker, never by extracting a value (see below); a present-but-different base is rejected, an absent one is not, since verdicts predating that convention carry none; every other shape, including any unrecognised one, is re-derived rather than trusted. The bot and docs-only exemptions are evaluated as INDEPENDENT predicates and the decision made afterwards, never as an `elif` chain. `edited` is in the workflow's `types:` so a retarget reclassifies — which gives DETECTION, not atomicity: status writes are not serialized, so a stale run can still post over a fresher one. That residual is now FENCED rather than merely tracked (NARROWED, not resolved — a retarget between the fence's final count and the POST still leaves a PERMANENT forged green, #849) — the job refuses to write at all if the PR's timeline retarget COUNT moved while it was classifying (`ci.verdict-write-retarget-fence`, #706) — leaving TWO windows, not one: the sub-round-trip window that no API without compare-and-set can close, AND the post-final-count/pre-POST window, which is not sub-round-trip and yields a PERMANENT green because the successor can consume the `edited` event and exit before the stale run posts last (#849). The PROTECTED path list additionally covers `CLAUDE.md` and `AGENTS.md` (#751) — they are not prose but the documents DEFINING the completion protocol, the merge-consent convention and the H10 rule, so protecting `.claude/` while the file specifying what it enforces stayed docs-only-exempt was the same self-exemption one directory over; driving the real classify body with a lone `CLAUDE.md` change produced an exemption `success`. `README.md` is deliberately not listed. It also covers `.codex/` (#711), which mirrors `.claude/hooks/` byte for byte including the merge-consent hook — latent while that directory is untracked, live the moment it is tracked; the list stays ENUMERATIVE rather than derived, because a derived rule would have to be evaluated against the very file list being classified. Reading the CURRENT status for input (3) must tolerate `statuses: null`: `GET /commits/{sha}/status` serialises a nil slice as `null`, not `[]`, on a head with no statuses yet, and an `array`-only gate made `read_existing_verdict` `exit 1` and post nothing at all (#751, `ci.workflow-run-body-no-expressions`) — `null` is accepted only when `total_count` is 0, so a body that merely lost its array is still refused. Path predicates are evaluated by COUNTING with `grep -c`, never `\| grep -q` (SIGPIPE inversion) and never a here-string (temp-space failure) — see `ci.grep-q-pipefail-inversion`. | 2026-07-29 | [link](records/ci/exemption-provenance.md) | | `ci.format-gate-folder-mode` | The blocking `format` CI job (and matching pre-commit hook) runs `dotnet format whitespace . --folder --include ` instead of loading the full MSBuild/Roslyn solution, cutting the gate from ~480s to ~0.5s with unchanged whitespace/charset coverage. | 2026-07-19 | [link](records/ci/format-gate-folder-mode.md) | | `ci.functional-e2e-harness` | The `functional-e2e` CI job boots the PR's own code from source via `dotnet run` (`scripts/e2e-local.sh`) and runs deterministic assertions (`scripts/e2e-functional.sh`) as an advisory (non-blocking) job, not a `build` dependency or required check. Originally curl-only; since #445 the same job carries a second, headless-browser step for the contracts curl cannot express — see `ci.ui-e2e-harness`. | 2026-07-16 | [link](records/ci/functional-e2e-harness.md) | -| `ci.gate-trigger-base-resolved` | The workflow that writes the branch-protection-required `review-verdict/h10` status triggers on `pull_request_target` with `branches: [main]`, never on plain `pull_request`. Gitea resolves a `pull_request` workflow DEFINITION from the PR's own head commit, so under that trigger a PR editing `.gitea/workflows/review-verdict.yml` ran its own rewritten copy and could post `h10=success` for itself; `pull_request_target` resolves the definition from the base instead. The `branches: [main]` filter is part of the rule, not a refinement of it: base resolution only relocates the rewrite from the head to the base, so without the filter a PR opened into an attacker-pushed base branch runs that branch's gate. `pull_request_target` is safe HERE only because this job never checks out or executes head-supplied code — it checks out `base.sha` and runs only that tree's scripts (`ci.shared-pr-file-enumeration`); reintroducing a head checkout under this trigger would be worse than the bug it fixed. This closes the rewrite route through THIS workflow and does NOT close the class: Gitea injects a write-capable `GITEA_TOKEN` into EVERY job, so any ref-resolved workflow — and a collaborator's own API token, since branch protection binds the context and not its issuer — can still forge `review-verdict/h10`. The credential half is now RESOLVED in `ci.actions-credential-scoping` (#697): CI's registry secret was the ADMIN account's basic auth and is now a PAT that cannot post a status, which removes the ADMIN escalation and that credential's route (an ALLOW-LISTED user credential's forgery carries a matching `creator` and is inherited as a verdict; an Actions job's carries `creator: null` and is re-derived, as since #742 is every account outside `H10_REVIEWERS` — but do NOT read that asymmetry as protection: re-derivation fires only on the trigger's `types`, and posting a status is not one of them, so a POST timed after the last PR event simply stands). It does not remove EVERY route: `RENOVATE_TOKEN` is a `write:repository` bot PAT in the same secret store, reachable by any PR-added workflow. The injected token stays write-capable until Gitea >=1.26 with a Restricted default (server-management#714), and a collaborator's own token remains unfixable; the exemption path has its own separate defects in #698. | 2026-07-28 | [link](records/ci/gate-trigger-base-resolved.md) | +| `ci.gate-trigger-base-resolved` | The workflow that writes the branch-protection-required `review-verdict/h10` status triggers on `pull_request_target` with `branches: [main]`, never on plain `pull_request`. Gitea resolves a `pull_request` workflow DEFINITION from the PR's own head commit, so under that trigger a PR editing `.gitea/workflows/review-verdict.yml` ran its own rewritten copy and could post `h10=success` for itself; `pull_request_target` resolves the definition from the base instead. The `branches: [main]` filter is part of the rule, not a refinement of it: base resolution only relocates the rewrite from the head to the base, so without the filter a PR opened into an attacker-pushed base branch runs that branch's gate. `pull_request_target` is safe HERE only because this job never checks out or executes head-supplied code — it checks out `base.sha` and runs only that tree's scripts (`ci.shared-pr-file-enumeration`); reintroducing a head checkout under this trigger would be worse than the bug it fixed. This closes the rewrite route through THIS workflow and does NOT close the class. Gitea injects `GITEA_TOKEN` into EVERY job and it USED to be write-capable everywhere, so any ref-resolved workflow could forge `review-verdict/h10`; since #748 (2026-08-27) all six workflows here declare `permissions:` — five `code: read`, and `review-verdict.yml`'s gate job `code: write`, deliberately, because it IS the gate (the five rest on the TOP-LEVEL form, which is INFERRED rather than probed — the gate job's JOB-LEVEL form is the one measured; see `ci.actions-credential-scoping`). That narrows the INJECTED token and only that: deliberately NO "only X can post a status" summary is stated here, because any such summary generalizes from the injected token, which `permissions:` scopes, to every credential a job can reach, which it does not touch — `RENOVATE_TOKEN` is in the same secret store and is referenced by `renovate.yml`, one of the five holding `code: read`. `ci-image.yml` was the last holdout — its unfiltered `push:` made it the worst-placed of the six — and #744 closed that trigger route (`ci.toolchain-image-publish-is-a-dispatch`) while #748 scoped its token. So the route is NARROWED, not closed, and the SHAPE is open — a newly added workflow declaring write, or omitting a declaration and inheriting the owner default, is a route again, which is why `test_pr_changed_files.py` asserts every tracked workflow declares a block. And a collaborator's own API token still can, since branch protection binds the context and not its issuer. The credential half is now RESOLVED in `ci.actions-credential-scoping` (#697): CI's registry secret was the ADMIN account's basic auth and is now a PAT that cannot post a status, which removes the ADMIN escalation and that credential's route (an ALLOW-LISTED user credential's forgery carries a matching `creator` and is inherited as a verdict; an Actions job's carries `creator: null` and is re-derived, as since #742 is every account outside `H10_REVIEWERS` — but do NOT read that asymmetry as protection: re-derivation fires only on the trigger's `types`, and posting a status is not one of them, so a POST timed after the last PR event simply stands). It does not remove EVERY route: `RENOVATE_TOKEN` is a `write:repository` bot PAT in the same secret store, reachable by any PR-added workflow. The owner-level Restricted default (server-management#714) remains a separate OPEN lever — its effect on a declared `code: write` is unmeasured, and the version half of that condition has been satisfied since the 1.25.4 -> 1.27.1 upgrade on 2026-08-05, so it is not something to wait on a Gitea release for. A collaborator's own token remains unfixable; the exemption path has its own separate defects in #698. | 2026-07-28 | [link](records/ci/gate-trigger-base-resolved.md) | | `ci.gitea-milestone-filter-noop` | Never filter issues with the server-side `?milestones=` parameter — fetch all open issues once and filter LOCALLY on each issue's `.milestone.title`. | 2026-07-21 | [link](records/ci/gitea-milestone-filter-noop.md) | | `ci.grep-q-pipefail-inversion` | In any script running under `set -o pipefail`, a security or classification predicate of the form `producer \| grep -q…` is FORBIDDEN: `grep -q` exits at its first match, the producer then takes SIGPIPE and exits 141 once the data exceeds the pipe buffer (~64K), so `pipefail` reports the pipeline as FAILED even though grep MATCHED — inverting the predicate exactly when the input is large. A here-string (`grep -q… <<< "$data"`) is ALSO forbidden: bash materialises a large here-string via temporary storage, so it fails when temp space is full or unwritable, and inside an `if`/`!` that failure flips the predicate the same way. COUNT instead — `n=$(printf '%s\n' "$data" \| grep -cE "$re")` — because `grep -c` drains stdin (no early exit, no SIGPIPE) over an ordinary pipe (no temp file). Read grep's status honestly: exit 1 means a zero count and is a legitimate answer, anything >1 is a real error. Evaluate the counts ONCE at TOP LEVEL, never inline inside an `if`/`elif` condition: inside `$( )` an `exit` leaves only the subshell and `set -e` does not fire, so an error silently reads as "no match". Validate that each result is numeric and fail closed if not. This applies to both the enforced gate `.gitea/workflows/review-verdict.yml` and the advisory hook `.claude/hooks/pretooluse-merge-consent.sh`. | 2026-07-29 | [link](records/ci/grep-q-pipefail-inversion.md) | | `ci.infra-shaped-red-under-load` | When a job dies inside a setup/cache step before your code compiles, check the runner host's load before diagnosing the diff, and never file a CI bug off one sample under pressure. | 2026-07-21 | [link](records/ci/infra-shaped-red-under-load.md) | diff --git a/docs/decisions/records/ci/actions-credential-scoping.md b/docs/decisions/records/ci/actions-credential-scoping.md index e1670365c..cf2ae2cb7 100644 --- a/docs/decisions/records/ci/actions-credential-scoping.md +++ b/docs/decisions/records/ci/actions-credential-scoping.md @@ -5,7 +5,7 @@ status: active since: '2026-08-05' supersedes: none superseded-by: none -rule: 'Any credential reachable from an Actions job is scoped to what that job needs. The container-registry secret `REGISTRY_PASSWORD` is a personal access token scoped `write:package` + `read:repository` — never an account PASSWORD. This matters because Gitea has NO `status` token scope: `POST /repos/{o}/{r}/statuses/{sha}` is gated by `reqRepoWriter(unit.TypeCode)`, so ANY credential that can write the repository can forge `review-verdict/h10`, the required context that is supposed to make merge-consent derived rather than assertable. Package-write IS a separate scope, so the registry credential can be made status-incapable at no cost: `scripts/ci-detect-already-validated.sh` only GETs. Do NOT add a `permissions:` key to constrain the injected `GITEA_TOKEN` on the assumption that it binds — below Gitea 1.26.0 it is silently a NO-OP, which is worse than absent because it reads in review as a constraint. That version precondition NO LONGER HOLDS: this instance was upgraded 1.25.4 -> 1.27.1 on 2026-08-05. What has NOT changed is that the consequence is unverified — whether `permissions:` is honored here is still UNPROBED, and there is still no API surface for it (`/api/v1/settings/actions` 404s at 1.27.1). The instance default HAS since been probed and is NOT unknown: it was set to Restricted, verified, and reverted to `permissive` on 2026-08-05 (#748), which is where it stands. Probe before relying on it; do not read the upgrade alone as the constraint now working. Scoping is necessary and not sufficient: it bounds what a job may DO, never whether attacker YAML runs at all, so a self-referencing trigger needs its own filter. That landed for `ci-image.yml` in #744 (`ci.toolchain-image-publish-is-a-dispatch`) — deliberately NOT bundled here, because until it also removed the file from `ci-image-pin`''s `expected`, editing it re-pointed that job at the editing commit and reddened a blocking check. A second, separate consequence of the same boundary: `actions/checkout` persists a write-capable credential into `.git/config` unless `persist-credentials: false` is set, and all 16 of this repo''s checkouts now set it — 15 in #746 and `ci-image.yml`''s in #744, once `ci.toolchain-image-publish-is-a-dispatch` removed the two mechanical reasons it was excluded; the convention is held with no exemption list by `scripts/tests/test_workflow_persist_credentials.py` (#835). Ordering is part of the rule: unmask the dependent fetches FIRST, because until then a credential regression presents as an empty changed-file set and a silently skipped check rather than a red job. This record closes ONE route. It does not close the class, and the later sections say exactly what survives — read them before citing this record as a mitigation.' +rule: 'Any credential reachable from an Actions job is scoped to what that job needs. The container-registry secret `REGISTRY_PASSWORD` is a personal access token scoped `write:package` + `read:repository` — never an account PASSWORD. This matters because Gitea has NO `status` token scope: `POST /repos/{o}/{r}/statuses/{sha}` is gated by `reqRepoWriter(unit.TypeCode)`, so ANY credential that can write the repository can forge `review-verdict/h10`, the required context that is supposed to make merge-consent derived rather than assertable. Package-write IS a separate scope, so the registry credential can be made status-incapable at no cost: `scripts/ci-detect-already-validated.sh` only GETs. `permissions:` on a workflow/job DOES bind on this instance — MEASURED 2026-08-27 on 1.27.1 by matched scratch-base probe PRs differing only in one unit, the block carried at JOB level on `set-verdict-status` (`code: write` posted the probe status; `code: read` 403ed the POST, curl exit 22, no status written), so all six workflows here now declare it (#748). Two properties that make it usable: the declaration is EXHAUSTIVE, not additive — a unit omitted is not granted — and it binds while the owner-level default is `permissive`, which is what makes the five `code: read` declarations effective TODAY rather than only after a flip. NOT established: what a declared `code: write` does under a RESTRICTED default. GitHub semantics let `permissions:` only narrow, never widen past the default, and if Gitea copies that, Restricted would cap the gate job at read and `review-verdict/h10` would stop being writable — the exact catastrophic case. The probe ran under `permissive` and CANNOT rule that out. Flip the owner default only behind the scratch-base probe re-run under Restricted (server-management#714, still open for this reason). The earlier form of this rule said the opposite ("do NOT add a `permissions:` key on the assumption that it binds — below Gitea 1.26.0 it is silently a NO-OP"); that was correct at 1.25.4 and is retained here so a reader meeting the old advice recognises it as superseded. There is still no API surface for the owner-level default (`/api/v1/settings/actions` 404s at 1.27.1). The instance default HAS since been probed and is NOT unknown: it was set to Restricted, verified, and reverted to `permissive` on 2026-08-05 (#748), which is where it stands. Probe before relying on it; do not read the upgrade alone as the constraint now working. Scoping is necessary and not sufficient: it bounds what a job may DO, never whether attacker YAML runs at all, so a self-referencing trigger needs its own filter. That landed for `ci-image.yml` in #744 (`ci.toolchain-image-publish-is-a-dispatch`) — deliberately NOT bundled here, because until it also removed the file from `ci-image-pin`''s `expected`, editing it re-pointed that job at the editing commit and reddened a blocking check. A second, separate consequence of the same boundary: `actions/checkout` persists whatever the job token can do into `.git/config` unless `persist-credentials: false` is set — read-only everywhere since #748 declared `permissions:` on all six workflows — and all 16 of this repo''s checkouts now set it — 15 in #746 and `ci-image.yml`''s in #744, once `ci.toolchain-image-publish-is-a-dispatch` removed the two mechanical reasons it was excluded; the convention is held with no exemption list by `scripts/tests/test_workflow_persist_credentials.py` (#835). Ordering is part of the rule: unmask the dependent fetches FIRST, because until then a credential regression presents as an empty changed-file set and a silently skipped check rather than a red job. This record closes ONE route. It does not close the class, and the later sections say exactly what survives — read them before citing this record as a mitigation.' signals: 'admin password in CI secrets, registry credential scope, ETV_STATUS_AUTH can write statuses, forge review-verdict/h10, head-resolved workflow holds credentials, persist-credentials on actions/checkout, credential left in .git/config, masked git fetch yields empty changed set, Gitea token scopes, no status scope, write:package vs write:repository, permissions key no-op, GITEA_TOKEN default read/write, Restricted default token permissions, orphan secret, deploy key in secret store, toolchain image overwrite, prod floating tag write · paths: `.gitea/workflows/docker-build.yml`, `.gitea/workflows/pr-checks.yml`, `.gitea/workflows/dependency-scan.yml`, `.gitea/workflows/ci-image.yml`, `.gitea/workflows/renovate.yml`, `scripts/ci-detect-already-validated.sh` · issues: #697, #672, #698, #742, #743, #746, #748, #835, #420, server-management#714' mechanics: 'PAT `ci-registry-scoped-697`, scopes `write:package,read:repository`, stored as repo Actions secret `REGISTRY_PASSWORD`; `REGISTRY_USER` remains `timothy`. Verified 2026-08-05 on Gitea 1.25.4: registry push of a probe tag SUCCEEDED; `GET /commits/{sha}/status` 200; `POST /statuses/{sha}` REFUSED HTTP 403 `token does not have at least one of required scope(s), required=[write:repository], token scope=write:package,read:repository`. Probe artifacts deleted, confirmed 404. NOT measured with this token: the `container:` pull, the buildcache write and the base-image pull. Those rest on Gitea''s scope model (write implies read per category, read at tag `v1.25.4`) — INFERRED. Note WHICH run proves which: only the `container:` pull is exercised by a PR. `cache-to`/`cache-from` and the base-image pull are confined to the `build` job, which carries `if: github.event_name != ''pull_request''`, so they are first exercised on the post-merge push to `main` — AFTER the merge gate has passed. A wrong inference there reddens main, not the PR.' --- @@ -107,6 +107,33 @@ after that condition expires, leaving the guard permanently blind to the file it docs, NOT verified (2026-08-05): `permissions:` landed in 1.26.0 (Gitea PR #36173); no `app.ini` lever at any version; Gitea rejects GitHub's `statuses`/`checks` scopes. +**MEASURED 2026-08-27 on 1.27.1 (#748) — this block is NOT covered by the 1.25.4 caveat below.** It +re-probes exactly ONE of that caveat's items, the `permissions:` question, and supersedes the +"whether `permissions:` is honored here is UNPROBED" claim. It does NOT discharge the caveat: the +scope enum, the `reqRepoWriter` gate and the `write:package` 403 are still 1.25.4-dated and +un-re-run. Method: a matched pair of scratch-base probe PRs, identical except for one permission +unit, the block carried **at job level on `set-verdict-status`** — the same level and job as the +shipped `review-verdict.yml`, so the gate's own shape is what was measured. + +- `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. + +What each observation licenses, kept separate. `issues: read` was exercised: both runs logged +`Retarget fence: 0 retarget event(s) ... (trusted=yes)`, which requires `count_retargets` to have +walked `/issues/{n}/timeline`. `pull-requests: read` was exercised: the positive control posted +`=success`, reachable only through `complete=yes`, which requires `scripts/pr-changed-files.sh` to +have paged `/pulls/{n}` and `/pulls/{n}/files` to a validated end — the fence and the start marker do +NOT show this, the `success` does. Both reached the classify step's start marker, so neither was the +#751 dropped-step shape. + +The negative control is the load-bearing half — without it a green positive is equally consistent +with `permissions:` being a no-op sitting beside a permissive default. + +NOT established: what a declared `code: write` receives under a RESTRICTED owner default (both +controls ran under `permissive`); and the TOP-LEVEL declaration form the other five workflows use, +which is inferred rather than measured. That inference fails safe (a top-level key Gitea ignored is a +security no-op, never a brick). Probe branches and PRs (#851, #852) deleted after reading. + **Version caveat — this record's measurements are pinned to 1.25.4, the instance is now 1.27.1.** The instance was upgraded mid-session on 2026-08-05 (#743). Everything above measured on 1.25.4 is therefore a *dated* claim, not a current one: the scope enum, the `reqRepoWriter` gate and the 403 @@ -118,13 +145,17 @@ re-probing. Re-verification of the 1.25.4-pinned claims across the CI docs is tr `write:repository` bot PAT in the same store and still cannot be scoped down, because Renovate needs repo write; what changed is that a verdict posted with it is no longer *inherited* — `renovate` is not in `H10_REVIEWERS`, so `review-verdict.yml` re-derives it (#742). It remains status-capable, so it can -still overwrite a status; it can no longer impersonate a reviewer. The injected `GITEA_TOKEN` is write-capable in every job; -only Gitea >=1.26 with the Actions default set to **Restricted** binds it (server-management#714). -Both halves of that condition have now been checked and it does not hold, because of the second one: -the version half IS satisfied (1.27.1), while the default was set to Restricted, verified, and -reverted to `permissive` on 2026-08-05 because this repo is not ready for it (#748). So the route is -open, and open for a known reason rather than an unprobed one — waiting on a further Gitea upgrade -would be waiting on the wrong thing. A +still overwrite a status; it can no longer impersonate a reviewer. The injected `GITEA_TOKEN` was write-capable in every job. +That is no longer the mechanism: #748 declared `permissions:` on all six workflows (2026-08-27), and +because the declaration binds under the current `permissive` default, five hold `code: read` and only +`review-verdict.yml`'s gate job holds write. So the route is narrowed NOW, without the owner-level +flip. The owner-level Restricted default (server-management#714) remains worth applying as the +instance-wide floor for repos that declare nothing — but it is NOT yet shown safe here: whether a +declared `code: write` survives a Restricted default is unmeasured, and if Gitea caps at the default +the way GitHub does, the flip would make the gate unwritable. What is NOT closed: `review-verdict.yml` +still holds repo-write, so the property is "only a workflow visibly opting into write can assert the +verdict", not "no workflow can". Scoping still bounds what a job may DO, never whether attacker YAML +RUNS. A collaborator's own token always can — and after #742 an ALLOW-LISTED collaborator's still is inherited, which is the residual the allow-list explicitly does not touch. `docker-build.yml` publishes `:prod` from a `v*` tag push and a tag may point at ANY commit — a prod image with no PR, review or status (tag protections are empty). diff --git a/docs/decisions/records/ci/gate-trigger-base-resolved.md b/docs/decisions/records/ci/gate-trigger-base-resolved.md index 1df318d2c..8cdb12dc6 100644 --- a/docs/decisions/records/ci/gate-trigger-base-resolved.md +++ b/docs/decisions/records/ci/gate-trigger-base-resolved.md @@ -5,7 +5,7 @@ status: active since: '2026-07-28' supersedes: none superseded-by: none -rule: 'The workflow that writes the branch-protection-required `review-verdict/h10` status triggers on `pull_request_target` with `branches: [main]`, never on plain `pull_request`. Gitea resolves a `pull_request` workflow DEFINITION from the PR''s own head commit, so under that trigger a PR editing `.gitea/workflows/review-verdict.yml` ran its own rewritten copy and could post `h10=success` for itself; `pull_request_target` resolves the definition from the base instead. The `branches: [main]` filter is part of the rule, not a refinement of it: base resolution only relocates the rewrite from the head to the base, so without the filter a PR opened into an attacker-pushed base branch runs that branch''s gate. `pull_request_target` is safe HERE only because this job never checks out or executes head-supplied code — it checks out `base.sha` and runs only that tree''s scripts (`ci.shared-pr-file-enumeration`); reintroducing a head checkout under this trigger would be worse than the bug it fixed. This closes the rewrite route through THIS workflow and does NOT close the class: Gitea injects a write-capable `GITEA_TOKEN` into EVERY job, so any ref-resolved workflow — and a collaborator''s own API token, since branch protection binds the context and not its issuer — can still forge `review-verdict/h10`. The credential half is now RESOLVED in `ci.actions-credential-scoping` (#697): CI''s registry secret was the ADMIN account''s basic auth and is now a PAT that cannot post a status, which removes the ADMIN escalation and that credential''s route (an ALLOW-LISTED user credential''s forgery carries a matching `creator` and is inherited as a verdict; an Actions job''s carries `creator: null` and is re-derived, as since #742 is every account outside `H10_REVIEWERS` — but do NOT read that asymmetry as protection: re-derivation fires only on the trigger''s `types`, and posting a status is not one of them, so a POST timed after the last PR event simply stands). It does not remove EVERY route: `RENOVATE_TOKEN` is a `write:repository` bot PAT in the same secret store, reachable by any PR-added workflow. The injected token stays write-capable until Gitea >=1.26 with a Restricted default (server-management#714), and a collaborator''s own token remains unfixable; the exemption path has its own separate defects in #698.' +rule: 'The workflow that writes the branch-protection-required `review-verdict/h10` status triggers on `pull_request_target` with `branches: [main]`, never on plain `pull_request`. Gitea resolves a `pull_request` workflow DEFINITION from the PR''s own head commit, so under that trigger a PR editing `.gitea/workflows/review-verdict.yml` ran its own rewritten copy and could post `h10=success` for itself; `pull_request_target` resolves the definition from the base instead. The `branches: [main]` filter is part of the rule, not a refinement of it: base resolution only relocates the rewrite from the head to the base, so without the filter a PR opened into an attacker-pushed base branch runs that branch''s gate. `pull_request_target` is safe HERE only because this job never checks out or executes head-supplied code — it checks out `base.sha` and runs only that tree''s scripts (`ci.shared-pr-file-enumeration`); reintroducing a head checkout under this trigger would be worse than the bug it fixed. This closes the rewrite route through THIS workflow and does NOT close the class. Gitea injects `GITEA_TOKEN` into EVERY job and it USED to be write-capable everywhere, so any ref-resolved workflow could forge `review-verdict/h10`; since #748 (2026-08-27) all six workflows here declare `permissions:` — five `code: read`, and `review-verdict.yml`''s gate job `code: write`, deliberately, because it IS the gate (the five rest on the TOP-LEVEL form, which is INFERRED rather than probed — the gate job''s JOB-LEVEL form is the one measured; see `ci.actions-credential-scoping`). That narrows the INJECTED token and only that: deliberately NO "only X can post a status" summary is stated here, because any such summary generalizes from the injected token, which `permissions:` scopes, to every credential a job can reach, which it does not touch — `RENOVATE_TOKEN` is in the same secret store and is referenced by `renovate.yml`, one of the five holding `code: read`. `ci-image.yml` was the last holdout — its unfiltered `push:` made it the worst-placed of the six — and #744 closed that trigger route (`ci.toolchain-image-publish-is-a-dispatch`) while #748 scoped its token. So the route is NARROWED, not closed, and the SHAPE is open — a newly added workflow declaring write, or omitting a declaration and inheriting the owner default, is a route again, which is why `test_pr_changed_files.py` asserts every tracked workflow declares a block. And a collaborator''s own API token still can, since branch protection binds the context and not its issuer. The credential half is now RESOLVED in `ci.actions-credential-scoping` (#697): CI''s registry secret was the ADMIN account''s basic auth and is now a PAT that cannot post a status, which removes the ADMIN escalation and that credential''s route (an ALLOW-LISTED user credential''s forgery carries a matching `creator` and is inherited as a verdict; an Actions job''s carries `creator: null` and is re-derived, as since #742 is every account outside `H10_REVIEWERS` — but do NOT read that asymmetry as protection: re-derivation fires only on the trigger''s `types`, and posting a status is not one of them, so a POST timed after the last PR event simply stands). It does not remove EVERY route: `RENOVATE_TOKEN` is a `write:repository` bot PAT in the same secret store, reachable by any PR-added workflow. The owner-level Restricted default (server-management#714) remains a separate OPEN lever — its effect on a declared `code: write` is unmeasured, and the version half of that condition has been satisfied since the 1.25.4 -> 1.27.1 upgrade on 2026-08-05, so it is not something to wait on a Gitea release for. A collaborator''s own token remains unfixable; the exemption path has its own separate defects in #698.' signals: 'workflow definition resolved from head, PR rewrites the gate that judges it, self-approve a required status check, pull_request_target vs pull_request, gate trigger branches filter, attacker-supplied base branch, how to test a change to review-verdict.yml, workflow not exercised by its own PR, gate edit goes live only on merge, required_approvals 0 does not bind an author, forged commit status inherited by sha · paths: `.gitea/workflows/review-verdict.yml`, `scripts/tests/test_pr_changed_files.py` · issues: #672, #663, #649, #622' mechanics: '`on: pull_request_target: {branches: [main], types: [opened, reopened, synchronize, ready_for_review, edited]}` (`edited` added by `ci.exemption-provenance` so a retarget reclassifies); asserted by `test_the_workflow_trigger_is_pull_request_TARGET_scoped_to_main` in `scripts/tests/test_pr_changed_files.py`; the job''s own context is renamed to `... (pull_request_target)` and must stay OUT of branch protection''s required list' --- @@ -50,9 +50,23 @@ authority the judged thing can assert is not a gate, not because an attack was e **The class is NOT closed, and this record must not be read as claiming otherwise.** This fixed one instance of "a ref-resolved workflow can obtain credentials that POST a commit status", and that -inventory is not a short list: Gitea injects `GITEA_TOKEN` into **every** job, defaulting to -read/**write**, so head-resolved, `push`-triggered and `workflow_dispatch` workflows alike are routes -(1.24+ loads a dispatched definition from the selected branch). A collaborator's own API token is a +inventory is not a short list. Gitea injects `GITEA_TOKEN` into **every** job, and it used to +default to read/**write**, so head-resolved, `push`-triggered and `workflow_dispatch` workflows alike +were routes (1.24+ loads a dispatched definition from the selected branch). Narrowed since #748 +(2026-08-27): all six workflows here now declare `permissions:` explicitly — five `code: read`, and +`review-verdict.yml`'s gate job `code: write`, deliberately, because it IS the gate. (The five rest +on the TOP-LEVEL declaration form, which is inferred rather than probed; the gate job's JOB-LEVEL +form is the one that was measured — see `ci.actions-credential-scoping`.) `ci-image.yml` was the last +holdout: its unfiltered `push:` trigger made it the worst-placed of the six, and #744 closed that +route (`ci.toolchain-image-publish-is-a-dispatch`) in the same week #748 scoped its token. + +**What that narrows is the INJECTED token, and only that.** No one-line "only X can post a status" +summary belongs here: every such summary quietly generalizes from the injected token, which +`permissions:` scopes, to every credential a job can reach, which it does not touch. The falsifying +case is concrete and current — `RENOVATE_TOKEN` still writes statuses and is referenced by +`renovate.yml`, one of the five holding `code: read`. The SHAPE is open too: a new workflow that +declares write, or omits a declaration and inherits the owner default, is a route again, which is why +`test_pr_changed_files.py` asserts every tracked workflow declares a block. A collaborator's own API token is a route with no workflow at all — branch protection binds the *context*, not its issuer. Full inventory in `#697`, whose credential half is resolved in `ci.actions-credential-scoping` — the registry secret no longer carries status-write. That does NOT leave the workflow routes provenance-free: any diff --git a/docs/decisions/records/release/main-direct-push-disabled.md b/docs/decisions/records/release/main-direct-push-disabled.md index 7ca5c0933..818998d89 100644 --- a/docs/decisions/records/release/main-direct-push-disabled.md +++ b/docs/decisions/records/release/main-direct-push-disabled.md @@ -32,7 +32,8 @@ the only admin exempts everybody.** Both fields are now set; treat them as one c cite `enable_push: false` alone as the reason the gate holds. **What this actually closes, and what it does not.** It closes the *write-only* credential routes, -which is most of #743's own "who can do it" list: the injected `GITEA_TOKEN` (repo write, not admin), +which is most of #743's own "who can do it" list: the injected `GITEA_TOKEN` (repo write at the time, +narrowed to `code: read` in five of six workflows by #748; not admin either way), `RENOVATE_TOKEN`, and any non-admin collaborator PAT. Those can no longer reach `main` at all, by any path that skips the gate. diff --git a/docs/decisions/records/release/verdict-status-check.md b/docs/decisions/records/release/verdict-status-check.md index c74ceec93..5b59d55bf 100644 --- a/docs/decisions/records/release/verdict-status-check.md +++ b/docs/decisions/records/release/verdict-status-check.md @@ -184,7 +184,9 @@ described as one: credentials that can POST a commit status can still forge `review-verdict/h10`; `docker-build.yml` demonstrably could, and must stay head-resolved because it builds the PR's own code — so #697 scoped its credential instead (`ci.actions-credential-scoping`), leaving AT LEAST - these: the injected `GITEA_TOKEN` (posts with `creator: null`), `RENOVATE_TOKEN` (a + these: the injected `GITEA_TOKEN` (posts with `creator: null`) — narrowed by #748, which declared + `permissions:` on all six workflows so only `review-verdict.yml`'s own gate job intentionally holds + repo-write, though that job's token remains status-capable by construction — `RENOVATE_TOKEN` (a `write:repository` bot PAT in the same secret store, so it posts with a real creator — it is no longer *inherited* as a verdict, since #742 replaced the "non-null creator" test with membership in the workflow's `H10_REVIEWERS` allow-list and `renovate` is not on it, but it can still overwrite diff --git a/docs/guard-inventory.md b/docs/guard-inventory.md index d92180ce9..113a5931c 100644 --- a/docs/guard-inventory.md +++ b/docs/guard-inventory.md @@ -133,7 +133,7 @@ The audit #806 asked for, recorded whichever way it came out, because "we looked | `test_guard_inventory.py` | `.claude/hooks/*.sh`, `.husky/*`, `scripts/tests/test_*.py`, workflow/hook callers | **yes** — set equality against this table | converted to the index; `.husky/_/` had been excluded only because `_` is a directory, so the obvious "make it recursive" edit would have reintroduced #778's defect here | | `test_hook_fire_log.py` | `.claude/hooks/*.sh` | **yes** — every hook must be instrumented | converted; an untracked scratch `.sh` used to demand instrumentation and redden the suite on that checkout alone | | `test_ci_image_pin_population.py` | `.gitea/workflows/*.yml` + `*.yaml` | **yes** — "docker-build is the ONLY workflow pinning the toolchain image" | converted, and `*.yaml` added: Gitea accepts both spellings, so a `.yaml` workflow was structurally invisible while the test read as covering all of them | -| `test_pr_changed_files.py` | `.gitea/workflows/*.y*ml` | **yes** — "no OTHER workflow writes the review-verdict status" | converted. Not on #806's list and found by cold review on the pushed head: an untracked `.yaml` dropped in `.gitea/workflows/` reddened two guards while absent from the index — the issue's list of files to assess was a starting point, not the population | +| `test_pr_changed_files.py` | `.gitea/workflows/*.y*ml` | **yes** — "no OTHER workflow writes the review-verdict status", and since ersatztv#748 "every tracked workflow declares a `permissions:` block", with NO exemption list (the one `ci-image.yml` briefly needed was deleted when #744 landed, per #835) | converted. Not on #806's list and found by cold review on the pushed head: an untracked `.yaml` dropped in `.gitea/workflows/` reddened two guards while absent from the index — the issue's list of files to assess was a starting point, not the population | | `test_workflow_persist_credentials.py` | `.gitea/workflows/*.yml` + `*.yaml` | **yes** — every `actions/checkout` in every workflow must drop the persisted credential | derived from the index from the start (#835). Shipped with NO exemption list, which is why it waited for #744: the one non-compliant checkout would otherwise have bought a permanent entry that keeps passing after its reason expires | | `web/src/api/pageSizeCallSites.guard.test.ts` | `web/src/**/*.{ts,tsx,mts,cts}` via `import.meta.glob` | **yes** — an unregistered discovered site fails | DEFERRED to #819, assessed not skipped. The glob is a documented workaround: `@types/node` is deliberately out of `tsconfig.app.json`, and wiring it in was tried and reverted (it leaked Node's `setTimeout` into the app project and broke three unrelated tests), so there is no `node:child_process` to reach the index from. Over-enumerates, so it fails loudly rather than going blind | | `test_ci_release_path_scan_job.py` | `.gitea/workflows/*.y*ml` + `scripts/**` | **no** — a fixture assembling a tmp harness, asserted about behaviour not membership | takes its file LIST from the index anyway, for hermeticity not completeness: `shutil.copytree` copied whatever was on disk, including untracked files and `scripts/__pycache__`, into a tree whose behaviour the probes then measure. Content still comes from the working tree. The copy is not a git repo, so the two files this step RUNS may not use the helper — see the fixture docstring | diff --git a/scripts/tests/test_pr_changed_files.py b/scripts/tests/test_pr_changed_files.py index 5f685c971..1f69999cf 100644 --- a/scripts/tests/test_pr_changed_files.py +++ b/scripts/tests/test_pr_changed_files.py @@ -3567,3 +3567,118 @@ def test_an_UNREADABLE_history_page_2_also_repairs_rather_than_leaving_green(tmp assert seq[0]["state"] == "success" and seq[1]["state"] == "pending", ( f"expected an exemption then a repair to pending; got {seq}" ) + + +# --- Workflow token scope (ersatztv#748) ------------------------------------------------------- +# +# Both assertions guard a property whose violation is SILENT and, for the first, unrecoverable. +# `permissions:` is exhaustive: dropping a unit does not degrade, it 403s the `curl -sf` under +# `set -euo pipefail`, so `review-verdict/h10` is never posted and EVERY merge to `main` blocks — +# including the PR that would repair it, with no force-merge escape since ersatztv#743. And because +# `review-verdict.yml` is base-resolved, the PR making that edit CANNOT catch it: its own run uses +# the definition already on `main`. A test on the tree is the only place this is checkable at review +# time. Measured basis for the unit list: docs/ci-cd.md -> "Workflow token scope". + + +def _declared_permissions(path, job=None): + import yaml + + doc = yaml.safe_load(path.read_text()) or {} + if job is None: + return doc.get("permissions") + return ((doc.get("jobs") or {}).get(job) or {}).get("permissions") + + +def test_the_verdict_gate_job_declares_exactly_the_units_its_api_calls_need(): + """The gate job's block is load-bearing in BOTH directions, so this pins the whole mapping. + + Not just `code: write`. A missing read unit is quieter but still wrong: without `issues: read` + the retarget fence cannot establish a trusted count and the exemption is withheld, and without + `pull-requests: read` the enumeration degrades to `complete=no` and every exempt PR falls to a + generic `pending`. Those are recoverable, unlike a missing `code: write`, but they silently turn + the two exemption classes off — the ersatztv#751 failure shape, arrived at from a different + direction. + + Equality, not containment: an EXTRA unit is a finding too. This job holds the only repo-write in + the repo's CI, so widening it (`contents: write`, a stray `packages:`) is exactly the drift the + scoping in ersatztv#697/#748 exists to prevent, and it would land unremarked as "just adding a + permission". + + WHAT THIS DOES NOT CATCH, stated because the test's name over-promises: it TRANSCRIBES the unit + set, it does not DERIVE it from the job's API calls. Add a new `gh` call to the workflow that + needs an undeclared unit and this test stays green while the gate breaks in production — the + failure this test exists for, arriving from the other side. Deriving the mapping would mean + parsing every URL out of a 1300-line shell body and knowing Gitea's endpoint-to-unit table, which + is a different and much larger change. So: when you add an API call to this job, the unit list + here is a checklist to revisit by hand, not a net that will tell you. (Same honesty as + `test_the_verdict_workflow_has_NO_expression_delimiter_in_any_run_body` above.) + """ + wf = REPO_ROOT / ".gitea" / "workflows" / "review-verdict.yml" + assert _declared_permissions(wf) is None, ( + "review-verdict.yml grew a TOP-LEVEL permissions: block. Only the job-level one on " + "set-verdict-status was probed (docs/ci-cd.md -> 'Workflow token scope'); adding a " + "top-level default changes what other jobs in this file would inherit and is unmeasured." + ) + assert _declared_permissions(wf, "set-verdict-status") == { + "code": "write", # POST /statuses/{sha} — Gitea has no `statuses` scope, so this IS it + "issues": "read", # GET /issues/{n}/timeline — the ersatztv#706 retarget fence + "pull-requests": "read", # scripts/pr-changed-files.sh -> /pulls/{n}, /pulls/{n}/files + }, ( + "the review-verdict gate job's permissions: block no longer matches the unit set that was " + "measured against its API calls. Dropping `code: write` makes review-verdict/h10 UNWRITABLE " + "and blocks every merge with no force-merge escape (ersatztv#743), and this workflow is " + "base-resolved so the PR changing it cannot detect that. Re-run the scratch-base probe " + "(docs/ci-cd.md -> Review-verdict gate) before changing this." + ) + + +def test_every_tracked_workflow_declares_a_permissions_block(): + """Otherwise the convention is prose only, and a NEW workflow is the case that breaks it. + + A workflow added without `permissions:` inherits the owner-level Actions default, which is + `permissive` today — i.e. a full read/write repository token, which is status-capable and can + therefore forge `review-verdict/h10` (`ci.actions-credential-scoping`). That is silent: nothing + reddens, the new file simply holds more than it needs. + + Population comes from the GIT INDEX via `_workflow_files()`, not a directory listing — same + reason as the two completeness claims above (ersatztv#806/#778). + + NO EXEMPTIONS, deliberately. `ci-image.yml` briefly needed one: editing that file re-pointed + `ci-image-pin`'s `expected` at the editing commit and reddened a BLOCKING job, and its own + `paths:` made the edit publish an image. ersatztv#744 took that path out of both + (`ci.toolchain-image-publish-is-a-dispatch`), so this asserts over the whole derived population + with nothing carved out — which is the form ersatztv#835 asked for. If a future file seems to + need an exemption, that is a finding about the file, not about this test. + """ + import yaml + + workflows = _workflow_files() + # Anti-vacuity. `tracked_children` matches direct children of one directory, so a renamed or + # moved `.gitea/workflows/` yields an EMPTY population and every completeness claim below passes + # having examined nothing — the #778 shape this test's own docstring invokes. `_git_ls_files`'s + # assert covers the whole index, not this subset, so it does not catch it. Measured: after + # `git mv .gitea/workflows .gitea/wf-renamed` this test passed with 0 files before this guard. + assert workflows, ( + "no tracked workflows found under .gitea/workflows — this test examined NOTHING and would " + "have passed vacuously. The directory was renamed/moved, or the glob no longer matches." + ) + + missing = [] + for wf in workflows: + doc = yaml.safe_load(wf.read_text()) or {} + jobs = (doc.get("jobs") or {}).values() + # Top-level absent is fine only if EVERY job declares its own — one undeclared job still + # inherits the owner default, so `any` would pass a file that is half-covered. + declares = doc.get("permissions") is not None or ( + bool(jobs) and all((job or {}).get("permissions") is not None for job in jobs) + ) + + if not declares: + missing.append(wf.name) + assert not missing, ( + f"these workflows declare no permissions: block at either level: {missing}. Every workflow " + "in this repo declares one (ersatztv#748), with NO exemption since #744 landed, so a new " + "file cannot silently inherit the " + "owner-level default. Read-only (`permissions: {code: read}` at top level) is the " + "convention; declare write only with a stated reason at the declaration." + )