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 8f39aef05..e93dc90f0 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 image that # ci-image.yml actually last published — i.e. the short sha of the last commit to touch the image's 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 28a4d3621..c31b8001b 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -147,14 +147,137 @@ 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:` **except `ci-image.yml`** (carve-out below). +Four 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 four declaring workflows use `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 four that declare a block 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.) +- `dependency-scan.yml` and `renovate.yml` (`schedule` / `workflow_dispatch` only) are **not** + exercised by an editing PR at all — their first run is on the next schedule. Neither 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. + +### The `ci-image.yml` carve-out — a scheduling constraint, not a safety judgement + +`ci-image.yml` declares **no** `permissions:` block, so it still receives the owner-level default and +its token is write-capable, as is the credential its checkout persists (it is also the one checkout +that does not set `persist-credentials: false`, ersatztv#746). That is the *worst*-placed of the six, +not the safest: its `push:` trigger has no `branches:` filter and is path-scoped to itself, so a push +touching `docker/ci/**` or that file — which a forging edit necessarily does — runs its head-resolved +YAML with a status-capable token, on any branch and with no PR. + +It is excluded for a scheduling reason. A PR editing that file re-points `ci-image-pin`'s `expected` +at the editing commit and staleness-fails a **blocking** job, and the same edit would publish a +toolchain image from the editing branch. Both are why +`docs/ci-cd.md` -> "CI toolchain image" documents the two-step this would otherwise force, which is +why the file lands alone. It rides with **ersatztv#744**, which is also the issue that closes the trigger +route — scoping this file's token would not have closed it anyway, because scoping bounds what a job +may *do* and only a filter bounds whether it *runs*. + +`scripts/tests/test_pr_changed_files.py` carries this as a one-name literal exemption with the same +reference. **ersatztv#835 tracks deleting that exemption — and #746's matching one — once #744 has +landed.** Adding a second name to it is a finding, not a fix. + ## Checkouts drop the persisted credential (ersatztv#746) Every `actions/checkout` in this repo **except `ci-image.yml`'s** (see the carve-out below) sets `persist-credentials: false`. 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. Since ersatztv#748 that default no longer governs MOST of this repo's jobs +— five of the six workflows declare `permissions:` explicitly (see the section above). But +`ci-image.yml` is not one of them: it is excluded from #748 for the same scheduling reason it was +excluded from #746, so it still receives the owner default and **the credential it persists is still +write-capable**. That is this section's carve-out, and #748 did not change it; both exclusions clear +together when ersatztv#744 lands. 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, @@ -1444,10 +1567,14 @@ as establishing that the gate cannot be forged (see the residual below, and ersa `pull_request` because it builds the PR's code, so it got the read-only status identity instead — its `ETV_STATUS_AUTH` is now a PAT scoped `write:package` + `read:repository`, which the status endpoint refuses (`ci.actions-credential-scoping`, ersatztv#697). The inventory was never that one - workflow, though: Gitea injects a write-capable `GITEA_TOKEN` into every job and branch protection - binds the *context*, not its issuer. Gitea >=1.26 with the Actions default set to **Restricted** - (server-management#714) binds the injected token, but does not close the class either — not against - a personal token, and not against `RENOVATE_TOKEN` (ersatztv#742). **And none of it was necessary: + workflow, though, and branch protection binds the *context*, not its issuer. Gitea used to inject + a write-capable `GITEA_TOKEN` into every job; since ersatztv#748 every workflow here declares + `permissions:` explicitly except `ci-image.yml`, so four hold `code: read` and only + `review-verdict.yml`'s gate job holds + write — see "Workflow token scope" above, which is the current statement of this. That narrows the + inventory; it does not close the class — not against a personal token, and not against + `RENOVATE_TOKEN` (ersatztv#742). The owner-level **Restricted** default (server-management#714) is + a separate, still-open lever whose effect on a declared `code: write` is unmeasured. **And none of it was necessary: direct pushes to `main` were server-side permitted, so the gate could be skipped without any forgery (ersatztv#743). That is now CLOSED — `main` carries `enable_push: false` **and** `block_admin_merge_override: true`, so it is reachable only through the PR merge path, the one path diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 05fd2adee..cb7dd7663 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 (`ci-image.yml`, tracked in #744 — deliberately NOT bundled here, because editing that file re-points `ci-image-pin` at the editing commit and reddens a blocking job). 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 15 of this repo's 16 checkouts now set it (#746), `ci-image.yml`'s being excluded for the reason in that section. 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 (`code: write` posted the probe status; `code: read` 403ed the POST, curl exit 22, no status written), so five of the six workflows here now declare it (#748); `ci-image.yml` is excluded and rides with #744, leaving it on the owner default. 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 four `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 therefore 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 only so a reader meeting the old advice recognises it as superseded rather than contradicted. 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 (`ci-image.yml`, tracked in #744 — deliberately NOT bundled here, because editing that file re-points `ci-image-pin` at the editing commit and reddens a blocking job). 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, and 15 of this repo's 16 checkouts now set it (#746), `ci-image.yml`'s being excluded for the reason in that section. #748 did NOT change that: it excluded `ci-image.yml` for the same scheduling reason, so that one persisted credential is still write-capable on the owner default. Both exclusions clear together when #744 lands. 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) five of the six workflows here declare `permissions:` and four hold `code: read`, so those cannot post a status (the top-level declaration form they use is INFERRED rather than probed — see `ci.actions-credential-scoping`). `ci-image.yml` is the EXCEPTION and it is exactly the dangerous shape: it declares nothing, so it still receives the owner default (write-capable), and its `push:` trigger has no `branches:` filter and is path-scoped to itself, so a push touching `docker/ci/**` or that file — which a forging edit necessarily does — runs its head-resolved YAML with a status-capable token, on any branch and with no PR. #748 deliberately did not touch it — editing that file re-points `ci-image-pin` at the editing commit and reddens a blocking job, so it rides with #744, which is the issue that closes this route. So the route is NARROWED, not closed: `ci-image.yml` is a current instance that remains open, and the SHAPE is open besides — a newly added workflow declaring write 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 542df3f1c..5d39ec505 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 (`ci-image.yml`, tracked in #744 — deliberately NOT bundled here, because editing that file re-points `ci-image-pin` at the editing commit and reddens a blocking job). 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 15 of this repo''s 16 checkouts now set it (#746), `ci-image.yml`''s being excluded for the reason in that section. 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 (`code: write` posted the probe status; `code: read` 403ed the POST, curl exit 22, no status written), so five of the six workflows here now declare it (#748); `ci-image.yml` is excluded and rides with #744, leaving it on the owner default. 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 four `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 therefore 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 only so a reader meeting the old advice recognises it as superseded rather than contradicted. 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 (`ci-image.yml`, tracked in #744 — deliberately NOT bundled here, because editing that file re-points `ci-image-pin` at the editing commit and reddens a blocking job). 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, and 15 of this repo''s 16 checkouts now set it (#746), `ci-image.yml`''s being excluded for the reason in that section. #748 did NOT change that: it excluded `ci-image.yml` for the same scheduling reason, so that one persisted credential is still write-capable on the owner default. Both exclusions clear together when #744 lands. 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.' --- @@ -23,7 +23,7 @@ with no PR. Scoping bounds what a job may DO; only a filter bounds whether it RU **#744**, not this record: editing `ci-image.yml` re-points `ci-image-pin`'s `expected` at the editing commit and staleness-fails a **blocking** job. That is a toll, not a wall — the documented two-step (publish `:`, then bump all five pins) clears it — but a rebase rewrites the sha and charges -it again, so it lands alone (`land-toolchain-image-change-separately`). +it again, so it lands alone (`docs/ci-cd.md` -> "CI toolchain image"). **What the scoped token still reaches — not "just a registry credential".** `write:package` over owner `timothy` writes `ersatztv:prod` (the floating tag prod's `jazz-media` stack follows) and @@ -72,9 +72,12 @@ reach the runner's docker daemon. **The persisted checkout credential (#746).** `actions/checkout` writes an `Authorization` header into `.git/config` unless `persist-credentials: false` is set, so a job hands that credential to -every script it runs, not only to the checkout — and it is write-capable while this instance's -Actions default token permission is `permissive` — where it stands after a Restricted trial was -reverted on 2026-08-05 (#748). 15 of the repo's 16 checkouts +every script it runs, not only to the checkout — and it was write-capable while this instance's +Actions default token permission is `permissive` — where that default still stands after a +Restricted trial was reverted on 2026-08-05. Since #748 the default no longer decides it: +`ci-image.yml` was deliberately left OUT of #748 (it rides with #744, same reason it was excluded +from #746), so it declares nothing, still receives the owner default, and the credential it persists +is still write-capable. It is the one file where the instance setting is what decides. 15 of the repo's 16 checkouts now set it: `docker-build.yml` (8), `pr-checks.yml` (5), `dependency-scan.yml` (1) and `review-verdict.yml` (1, pre-existing). No job that checks out pushes with git, so none of them loses a capability; `renovate.yml` does push, but has no checkout step and uses `RENOVATE_TOKEN`. @@ -110,17 +113,50 @@ probe were all taken pre-upgrade and have NOT been re-run. They are recorded hon date and are the best evidence available, but do not cite them as current behaviour without re-probing. Re-verification of the 1.25.4-pinned claims across the CI docs is tracked separately. +**MEASURED 2026-08-27 on 1.27.1 (#748) — this block is NOT covered by the 1.25.4 caveat above.** 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 still +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 the +same 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 from what it does not. `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`, 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. Both runs +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 by this probe: what a declared `code: write` receives under a RESTRICTED owner +default (both controls ran under `permissive`) — see the caveat in `rule`; and the TOP-LEVEL +declaration form the other four declaring 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), and +`docker-build.yml`/`pr-checks.yml` are `on: pull_request`, so their form is exercised by any PR +touching them. Probe branches and PRs (#851, #852) deleted after reading. + **Surviving routes — this record is not a mitigation for any of them.** `RENOVATE_TOKEN` is a `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 five of the six workflows (2026-08-27); `ci-image.yml` is excluded and rides with #744. +Because the declaration binds under the current `permissive` default, FOUR of them now hold +`code: read` and only `review-verdict.yml`'s gate job holds write (the fifth declarer). 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. That is the +remaining probe, not a formality. 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..f1d8e229b 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) five of the six workflows here declare `permissions:` and four hold `code: read`, so those cannot post a status (the top-level declaration form they use is INFERRED rather than probed — see `ci.actions-credential-scoping`). `ci-image.yml` is the EXCEPTION and it is exactly the dangerous shape: it declares nothing, so it still receives the owner default (write-capable), and its `push:` trigger has no `branches:` filter and is path-scoped to itself, so a push touching `docker/ci/**` or that file — which a forging edit necessarily does — runs its head-resolved YAML with a status-capable token, on any branch and with no PR. #748 deliberately did not touch it — editing that file re-points `ci-image-pin` at the editing commit and reddens a blocking job, so it rides with #744, which is the issue that closes this route. So the route is NARROWED, not closed: `ci-image.yml` is a current instance that remains open, and the SHAPE is open besides — a newly added workflow declaring write 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,22 @@ 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): five of the six workflows here now declare `permissions:` explicitly and four hold +`code: read`, so those cannot POST a status (that rests on the TOP-LEVEL declaration form, which is +inferred rather than probed — only the job-level form was measured; see +`ci.actions-credential-scoping`). **`ci-image.yml` is NOT covered and is the worst-placed of the +six**: it declares nothing, still gets the owner default, and its unfiltered `push:` trigger is +path-scoped to itself, so a push touching `docker/ci/**` or that file — which a forging edit +necessarily does — runs its head-resolved YAML with a status-capable token, on any branch, with no PR. +That is #744's route to close, not this one's — #748 could not touch the file without reddening a +blocking job (see `docs/ci-cd.md` -> "CI toolchain image" for the two-step it would force). So the +route is NARROWED, not closed: four of today's workflows can no longer post a status, `ci-image.yml` +still can, and the SHAPE is open besides — 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 576bed3af..93d972ec2 100644 --- a/docs/decisions/records/release/main-direct-push-disabled.md +++ b/docs/decisions/records/release/main-direct-push-disabled.md @@ -32,7 +32,9 @@ 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 four of six workflows by #748, `ci-image.yml` still on the owner +default; 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..652013855 100644 --- a/docs/decisions/records/release/verdict-status-check.md +++ b/docs/decisions/records/release/verdict-status-check.md @@ -184,7 +184,10 @@ 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 five of the six workflows (`ci-image.yml` rides with #744) 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 16d7ae765..e65db99ba 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" (both over the same git-derived population; the second carries ONE literal exemption, `ci-image.yml`, which rides with #744 — ersatztv#835 tracks deleting it) | 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 | | `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 | | `test_ci_dropped_step_guard.py` | the parsed workflow document | **no filesystem population at all** | unchanged; its residual is `MARKED_JOBS`, a SCOPE mirror of the required contexts on `main`, which #806 does not close | diff --git a/scripts/tests/test_pr_changed_files.py b/scripts/tests/test_pr_changed_files.py index 5f685c971..196c09262 100644 --- a/scripts/tests/test_pr_changed_files.py +++ b/scripts/tests/test_pr_changed_files.py @@ -3567,3 +3567,140 @@ 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). + + ONE EXEMPTION, and it is a scheduling constraint rather than a judgement that the file is safe. + `ci-image.yml` is excluded because a PR that edits it re-points `ci-image-pin`'s `expected` at the + editing commit and staleness-fails a BLOCKING job, and because its unfiltered `push:` trigger is + path-scoped to itself, so the same edit would publish a toolchain image from the editing branch. + That is why it lands alone (`docs/ci-cd.md` -> "CI toolchain image" documents the two-step an + edit would otherwise force), and it rides with ersatztv#744. This is the same exemption + `ci.actions-credential-scoping` already carries for that file's `persist-credentials`, and + ersatztv#835 tracks removing BOTH once #744 has landed — at + which point delete this set rather than adding to it. Adding a second name here is a finding, not + a fix: the exemption is one file for one dated reason, not a mechanism. + """ + # Deliberately a literal, not a pattern: a glob would silently absorb a future file. + EXEMPT = {"ci-image.yml"} + 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) + ) + + # EXEMPT is tested BEFORE `declares`, deliberately. The obvious order (skip anything that + # declares, then handle exemptions) makes the assert below dead code in exactly the case it + # exists for — measured: with that order, giving ci-image.yml a block left this test GREEN. + if wf.name in EXEMPT: + # SELF-DELETING. Without this, the day #744 lands and this file gains a block, the + # exemption silently becomes a permanent hole that only #835 remembers — and a follow-up + # issue goes stale exactly like a doc. Redden instead, so the tree tells you. + assert not declares, ( + f"{wf.name} is on this test's EXEMPT list but now DECLARES a permissions: block. The " + "exemption exists only because that file could not be edited without reddening " + "ci-image-pin (it rides with ersatztv#744). If #744 has landed, delete it from " + "EXEMPT here and drop the matching carve-outs in docs/ci-cd.md and " + "ci.actions-credential-scoping — ersatztv#835 tracks exactly that." + ) + continue + + 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) — except ci-image.yml, which rides with #744 — 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." + )