diff --git a/.gitea/workflows/review-verdict.yml b/.gitea/workflows/review-verdict.yml index 715415caf..eccb2fb46 100644 --- a/.gitea/workflows/review-verdict.yml +++ b/.gitea/workflows/review-verdict.yml @@ -471,10 +471,12 @@ jobs: # --- Whose verdict is it? (ersatztv#698 route 3) ------------------------------------- # This short-circuit used to exit on ANY existing `success`, which made an exemption this job # wrote indistinguishable from a verdict a human wrote. That is what let a forged exemption - # survive: obtained once — via the route-1 retarget race, a sibling workflow holding - # status-write credentials (#697), a direct API call, or inheritance across PRs by sha (#663) - # — it was thereafter accepted unchanged on every run, because the guard exited before it - # looked at the PR, the base, the author or the files. + # survive: obtained once — via the route-1 retarget race, a collaborator's own token, the + # `GITEA_TOKEN` injected into every job, `RENOVATE_TOKEN`, a direct API call, or inheritance + # across PRs by sha (#663) — it was thereafter accepted unchanged on every run, because the + # guard exited before it looked at the PR, the base, the author or the files. (`#697`'s + # registry credential can no longer post a status at all — see `ci.actions-credential-scoping` + # — but that closes only one route; the others above still work.) # # The guard still exists for its original reason: re-posting `pending` over a real human # verdict would un-approve a reviewed head and stall the PR. So it discriminates by PROVENANCE. @@ -491,9 +493,11 @@ jobs: # machine-written") an unrecognised shape would be trusted — the fail-open this issue is about. # # What this does NOT claim: the test asks "was this POSTed by a user credential", NOT "by a - # reviewer". `ETV_STATUS_AUTH` is basic auth, so head-controlled code can POST a success with a - # non-null creator AND an attacker-chosen `Review-verdict:` description, which this guard then - # preserves. That is #697 — provenance, not authentication. + # reviewer". `ETV_STATUS_AUTH`'s registry credential can no longer POST a status at all (#697, + # fixed by scoping it off `write:repository` — see `ci.actions-credential-scoping`), but any + # OTHER user credential — a collaborator's own token, `RENOVATE_TOKEN` — can still POST a + # success with a non-null creator AND an attacker-chosen `Review-verdict:` description, which + # this guard then preserves. That is provenance, not authentication. read_existing_verdict if [ "$ex_human" = yes ] && { [ "$ex_state" = "success" ] || [ "$ex_state" = "failure" ]; }; then echo "${CONTEXT} is already '${ex_state}' on ${SHA:0:7}, written by '${ex_creator}' as a human verdict — leaving it alone." diff --git a/docs/ci-cd.md b/docs/ci-cd.md index e35fcc6cb..8479c8e39 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -417,7 +417,10 @@ the image build. password** — deliberately, so head-resolved PR code cannot use it to forge a commit status (`ci.actions-credential-scoping`, ersatztv#697). If a job ever fails with `token does not have at least one of required scope(s)`, the fix is to narrow what the job does, **never** to widen the - token to `write:repository` or to put the admin password back. + token to `write:repository` or to put the admin password back. Note what the scope still reaches: + `write:package` covers `ersatztv:prod` (the tag prod's stack follows) and `ersatztv-ci:` (the + toolchain image five `container:` jobs execute), so this is the deployment supply chain, not an + inert endpoint — see `ci.actions-credential-scoping`. 4. `docker/build-push-action@v6`: amd64-only, `docker/Dockerfile`, `INFO_VERSION` build-arg, registry layer cache (`type=registry,ref=…:buildcache`, `cache-to … ignore-error=true`). @@ -846,8 +849,11 @@ forgery through a **user** credential inherits as a human verdict, while one thr was a user credential — the admin account — and no longer carries status-write. **`RENOVATE_TOKEN` still is one** (`write:repository`, a real bot account), and secrets are a per-repo store any PR-added workflow can reference, so that route is narrowed rather than closed; tightening this check -from "non-null creator" to an allow-list of approved reviewers is what would close it. A -collaborator's own personal token still can, and no repo-side change closes that. +from "non-null creator" to an allow-list of approved reviewers is what would close it +(ersatztv#742). A collaborator's own personal token still can, and no repo-side change closes that. +Note also that re-derivation is **not** a race the attacker can lose: it 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 stands +until the next one. Deciding either exemption requires the PR's **complete** changed-file list, which the workflow does not compute itself: it calls `scripts/pr-changed-files.sh`, the single shared implementation also @@ -919,8 +925,12 @@ as establishing that the gate cannot be forged (see the residual below, and ersa 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, so the class closes only at Gitea >=1.26 with the Actions - default set to Restricted (server-management#714) — and not even then against a personal token. The exemption path has separate defects of its own (ersatztv#698). One operational + 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 is necessary: + direct pushes to `main` are server-side permitted, so the gate can be skipped without any forgery + (ersatztv#743) — a push whitelist would close more of this class than the upgrade does.** The + exemption path has separate defects of its own (ersatztv#698). One operational consequence of the trigger change: a PR whose base is not `main` now gets **no** `review-verdict/h10` at all. That is fail-closed. `edited` **is** now among the trigger's `types` (ersatztv#698), so a PR retargeted onto `main` reclassifies instead of staying statusless until its diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 883552323..029ca7310 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -36,7 +36,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 by an Actions job is scoped to exactly what that job needs. The container-registry secret `REGISTRY_PASSWORD` is a personal access token scoped `write:package` + `read:repository` — never an account password, and never the admin account. 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 branch-protection-required context that makes 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 ever GETs a status, which `read:repository` covers. Do NOT add a `permissions:` key to a workflow to constrain the injected `GITEA_TOKEN` while this instance is below Gitea 1.26.0 — the key is silently a NO-OP there, which is worse than absent because it reads in review as a constraint. This record closes the instance-wide ADMIN escalation and the admin-credential forgery route; it does NOT close the class. Two things survive it: every job still receives a full read/write `GITEA_TOKEN`, and `RENOVATE_TOKEN` is still a `write:repository` PAT of a real bot account sitting in the SAME repo secret store, so it too can post a status with non-null `creator`. The secret STORE is the boundary, not any one workflow: a PR-added workflow can reference any secret in it. See the residual below. | 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. Do NOT add a `permissions:` key to constrain the injected `GITEA_TOKEN` while this instance is below Gitea 1.26.0 — it is silently a NO-OP there, which is worse than absent because it reads in review as a constraint. 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). This record closes ONE route. It does not close the class, and four 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. | 2026-07-21 | [link](records/ci/cancelled-is-not-a-verdict.md) | @@ -47,7 +47,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 only for a status POSITIVELY identified as a human verdict for THIS base, meaning a non-null `.creator.login` 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 — 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 only the sub-round-trip window that no API without compare-and-set can close. The PROTECTED path list additionally 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. 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 durable route (a user credential's forgery carries a real `creator` and is inherited as a human verdict; an Actions job's carries `creator: null` and is re-derived). It does not remove EVERY durable 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 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 (a user credential's forgery carries a real `creator` and is inherited as a human verdict; an Actions job's carries `creator: null` and is re-derived — 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.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 65fab5195..6b0b296bf 100644 --- a/docs/decisions/records/ci/actions-credential-scoping.md +++ b/docs/decisions/records/ci/actions-credential-scoping.md @@ -1,70 +1,74 @@ --- key: ci.actions-credential-scoping -title: '2026-08-05 — CI''s registry credential is a PAT scoped `write:package` + `read:repository`, never the admin account, because Gitea cannot separate status-write from repo-write (#697)' +title: '2026-08-05 — CI''s registry credential is a scoped PAT, not the admin password, because Gitea cannot separate status-write from repo-write (#697)' status: active since: '2026-08-05' supersedes: none superseded-by: none -rule: 'Any credential reachable by an Actions job is scoped to exactly what that job needs. The container-registry secret `REGISTRY_PASSWORD` is a personal access token scoped `write:package` + `read:repository` — never an account password, and never the admin account. 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 branch-protection-required context that makes 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 ever GETs a status, which `read:repository` covers. Do NOT add a `permissions:` key to a workflow to constrain the injected `GITEA_TOKEN` while this instance is below Gitea 1.26.0 — the key is silently a NO-OP there, which is worse than absent because it reads in review as a constraint. This record closes the instance-wide ADMIN escalation and the admin-credential forgery route; it does NOT close the class. Two things survive it: every job still receives a full read/write `GITEA_TOKEN`, and `RENOVATE_TOKEN` is still a `write:repository` PAT of a real bot account sitting in the SAME repo secret store, so it too can post a status with non-null `creator`. The secret STORE is the boundary, not any one workflow: a PR-added workflow can reference any secret in it. See the residual below.' -signals: 'admin password in CI secrets, registry credential scope, ETV_STATUS_AUTH can write statuses, forge review-verdict/h10, head-resolved workflow holds credentials, Gitea token scopes, no status scope, write:package vs write:repository, permissions key no-op, GITEA_TOKEN default read/write, Restricted default token permissions, PR code gets repo secrets · paths: `.gitea/workflows/docker-build.yml`, `.gitea/workflows/ci-image.yml`, `.gitea/workflows/renovate.yml`, `scripts/ci-detect-already-validated.sh` · issues: #697, #672, #698, #420, server-management#714' -mechanics: 'PAT `ci-registry-scoped-697` under `timothy`, scopes `write:package,read:repository`, stored as the 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` SUCCEEDED, `POST /statuses/{sha}` REFUSED with HTTP 403 `token does not have at least one of required scope(s), required=[write:repository], token scope=write:package,read:repository`. Probe package version and host artifacts deleted and confirmed 404 afterwards.' +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` while this instance is below Gitea 1.26.0 — it is silently a NO-OP there, which is worse than absent because it reads in review as a constraint. 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). This record closes ONE route. It does not close the class, and four 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, 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/ci-image.yml`, `.gitea/workflows/renovate.yml`, `scripts/ci-detect-already-validated.sh` · issues: #697, #672, #698, #742, #743, #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.' --- -**What was actually wrong.** `REGISTRY_USER`/`REGISTRY_PASSWORD` were the **admin account's** basic -auth, and `docker-build.yml` triggers on `pull_request` — head-resolved — so a PR's own code was -handed instance-admin credentials. Basic auth carries no scope: the same secret that pushes an image -administers every repo on the instance. `ci-image.yml` is worse in one respect and reached the same -credential: its `push:` trigger has **no `branches:` filter** and is path-scoped to *itself*, so -pushing any branch that edits that file runs attacker-supplied YAML — no PR required. +**What was wrong.** `REGISTRY_USER`/`REGISTRY_PASSWORD` were the **admin account's** basic auth, and +`docker-build.yml` triggers on `pull_request` — head-resolved — so a PR's own code got instance-admin +credentials. Basic auth carries no scope: the secret pushing an image administers every repo on the +instance. -**Why the fix is the credential and not the triggers.** Patching triggers one at a time enumerates -instances of "a ref-resolved workflow obtains status-capable credentials"; the inventory is every -workflow, and adding a *new* `.gitea/workflows/*.yml` is itself a route. Scoping the credential -degrades all three known routes at once — and any future one — from "instance admin + forgeable -gate" to "can push a container image", without touching a trigger or losing `ci-image.yml`'s -publish-from-a-branch capability. +**Why the credential and not only the triggers.** Patching triggers enumerates *instances* of "a +ref-resolved workflow obtains status-capable credentials", and adding a new workflow file is itself a +route, so that enumeration never completes. But it is not either/or: `ci-image.yml`'s unfiltered +`push:` is path-scoped to itself, so any branch push runs attacker YAML on a docker-capable runner +with no PR. Scoping bounds what a job may DO; only a filter bounds whether it RUNS. That filter is +**#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`). -**Why this closes the DURABLE route specifically.** Forgeries differ by provenance, and provenance is -what decides whether one survives. `review-verdict.yml` leaves an existing `h10` alone only when it is +**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 +`ersatztv-ci:` (the toolchain image *executing* five `container:` jobs). A sha-named tag is not an +immutable artifact (no container tag immutability in Gitea 1.25 — INFERRED), so overwriting the pinned +tag is code execution inside CI, chaining back into the routes below. This is the deployment supply +chain for prod and CI itself. + +**Admin ownership is a real residual.** The PAT is minted under `timothy`, a site admin. The 403 proves +the scope gate binds the *status* endpoint ahead of any admin bypass; it does NOT establish that for +*package* endpoints, where Gitea resolves permission by owner and an admin passes object-level checks, +so the token's package reach is plausibly wider than this repo. A non-admin bot account would close +this, but is not free: packages live in a user namespace only its owner and admins can write. Both +halves INFERRED, neither probed. + +**Provenance, corrected.** `review-verdict.yml` leaves an existing `h10` alone only when it is positively identifiable as human — non-null `.creator.login` plus a `Review-verdict:` description -(`release.verdict-status-check`). A status POSTed with a *user* credential carries a real creator, so -a forgery through the old admin secret was **inherited as a genuine human verdict**. One POSTed by an -Actions job carries `"creator": null` and is re-derived on the next PR event, so the `GITEA_TOKEN` -route must win a race. Removing status-write from the registry credential therefore removes *that* -durable route — **but not every one.** `RENOVATE_TOKEN` is a `write:repository` PAT belonging to the -dedicated `renovate` bot, so a status posted with it also carries a non-null `creator` and is also -inherited as a human verdict. It cannot be scoped down: Renovate genuinely needs repo write to push -branches and open PRs. And because secrets are a per-repo STORE rather than a per-workflow grant, a -PR-added workflow can reference it. The durable route is therefore narrowed, not closed. Closing it -needs the provenance check tightened from "non-null `creator`" to "`creator` in an allow-list of -approved reviewers", which is expressible in `review-verdict.yml` — deliberately NOT done here, -because an edit to the gate goes live only on merge, repo-wide and untested. +(`release.verdict-status-check`). A user credential posts with a real creator and is INHERITED; an +Actions job posts `creator: null` and is re-derived. **That asymmetry is not protection.** Re-derivation +fires only on `opened|reopened|synchronize|ready_for_review|edited`, and posting a status is none of +them, so a POST timed after the last event stands until the attacker merges. The gain here is that PR +code can no longer escalate to instance admin — NOT that the durable forgery route is closed. -**Audit the secret STORE, not the workflow set.** The same sweep found `SERVERMGMT_DEPLOY_KEY` still -present with no workflow referencing it — the `bump-prod-compose` job that used it was removed in -`1b5efd7b9`. An orphan secret is pure liability: an SSH deploy key to another repo, obtainable by any -workflow a PR adds, with no remaining benefit to offset it. Enumerating workflows would never have -found it; enumerating secrets did. +**The boundary is everything reachable from a job, not the secret store.** The store is a useful lower +bound — auditing it rather than the workflow set is what found `RENOVATE_TOKEN` and +`SERVERMGMT_DEPLOY_KEY` below, since any PR-added workflow can reference any secret. But +`GITEA_TOKEN` is injected and never in the store; nor is the credential +`actions/checkout` persists into `.git/config` (`docker-build.yml` omits `persist-credentials: false`); +and jobs reach the runner's docker daemon. -**Measured, not inferred.** The premise is a claim about someone else's software, so each half was -settled against this instance rather than by analogy to GitHub: the scope enum at tag `v1.25.4` -(`models/auth/access_token_scope.go`) has no `status` entry; the status endpoint's `reqRepoWriter` -gate was read at `routers/api/v1/api.go`; and the replacement credential was exercised end-to-end — -push, status read, status write — with the refusal quoted verbatim in `mechanics`. Gitea documents -that it does **not** support GitHub's separate `statuses`/`checks`/`deployments` scopes. +**Measured vs inferred.** Measured here: the `v1.25.4` scope enum (`access_token_scope.go`) has no +`status` entry; the `reqRepoWriter` gate (`routers/api/v1/api.go`); the probes in `mechanics`. Read from +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. -**The class is NOT closed, and this record must not be read as claiming otherwise.** Gitea injects -`GITEA_TOKEN` into every job with full read/write repository on 1.25.4, so any workflow can still -post the context; it simply cannot do so with human provenance. The only real boundary is Gitea -**≥1.26.0** with the org/repo Actions default set to **Restricted** — the `permissions:` YAML key -landed in 1.26.0 (PR #36173) and is inert before it, and there is no `app.ini` lever at any version: -the default lives in DB-backed repo/org settings. Tracked in **server-management#714**. Note that -even after the upgrade, a per-workflow `permissions:` key is not a boundary — a new workflow file can -omit it, so only the Restricted *default* binds. - -**The unfixable residual.** Branch protection binds the required *context name*, not its issuer, and -Gitea offers no way to restrict who may post a context. Anyone holding a write-scoped personal token -can POST `review-verdict/h10=success` directly, with genuine human provenance and no workflow at all. -**`h10` is a process guard against mistakes and drift, not a security boundary against push access.** -Severity throughout is bounded by that: never remotely exploitable, and the threat model is a -compromised contributor who already has other paths. +**Surviving routes — this record is not a mitigation for any of them.** `RENOVATE_TOKEN` is a +`write:repository` bot PAT in the same store, posting with a real creator, and cannot be scoped down +because Renovate needs repo write (#742). 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). A +collaborator's own token always can. `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). +**And none of it is necessary: direct pushes to `main` are server-side permitted, so the gate is +bypassable with no forgery at all (#743)** — a push whitelist closes more of this class than the 1.26 +upgrade. Treat this list as "at least these", never exhaustive. `SERVERMGMT_DEPLOY_KEY` remains in the +store though its `bump-prod-compose` job went in `1b5efd7b9`, and its key on `timothy/server-management` +is `read_only: false` — write access to the repo holding prod's GitOps stack definitions. Left in place +by explicit decision 2026-08-05; recorded so it is accepted, not forgotten. Severity throughout: push +access required, so a compromised contributor or subverted automated session, never an anonymous one. diff --git a/docs/decisions/records/ci/exemption-provenance.md b/docs/decisions/records/ci/exemption-provenance.md index 2be90abe7..9e0702b6a 100644 --- a/docs/decisions/records/ci/exemption-provenance.md +++ b/docs/decisions/records/ci/exemption-provenance.md @@ -79,9 +79,10 @@ test still fails: the guard degrades toward re-deriving, never toward trusting. **What this does NOT close.** Anyone who can POST statuses directly can write both a creator and a `Review-verdict:` description and impersonate a verdict; branch protection binds the *context*, not its -issuer. A provenance check, not an authentication one — that is `#697`, left open because its durable -fix is credential scoping, partly server-management territory. Severity as `#672`: requires push -access, so the threat model is a compromised contributor. +issuer. A provenance check, not an authentication one — that was `#697`'s registry credential, fixed by +scoping it off `write:repository` (`ci.actions-credential-scoping`); `GITEA_TOKEN`, `RENOVATE_TOKEN`, +and a collaborator's own token still can. Severity as `#672`: requires push access, so the threat model +is a compromised contributor. **Verification honesty.** Route 1 was reproduced live; the "and now it fails" half cannot be shown from a PR, because `pull_request_target` resolves this definition from `main` — the self-test gap diff --git a/docs/decisions/records/ci/gate-trigger-base-resolved.md b/docs/decisions/records/ci/gate-trigger-base-resolved.md index 64a837c97..f3a8c81f5 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 durable route (a user credential''s forgery carries a real `creator` and is inherited as a human verdict; an Actions job''s carries `creator: null` and is re-derived). It does not remove EVERY durable 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 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 (a user credential''s forgery carries a real `creator` and is inherited as a human verdict; an Actions job''s carries `creator: null` and is re-derived — 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.' 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' --- @@ -55,7 +55,9 @@ read/**write**, so head-resolved, `push`-triggered and `workflow_dispatch` workf (1.24+ loads a dispatched definition from the selected branch). 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, so no workflow route retains *human* provenance; the exemption path's +no longer carries status-write. That does NOT leave the workflow routes provenance-free: any +PR-added workflow can reference `RENOVATE_TOKEN`, a `write:repository` bot PAT in the same store, +whose status carries a real creator and IS inherited (`#742`). The exemption path's own defects are `#698`. No in-repository test can establish status-authority isolation: the sibling guard added here catches only plain-text naming of the context. diff --git a/docs/decisions/records/release/verdict-status-check.md b/docs/decisions/records/release/verdict-status-check.md index fcaf22732..00a7803d5 100644 --- a/docs/decisions/records/release/verdict-status-check.md +++ b/docs/decisions/records/release/verdict-status-check.md @@ -183,10 +183,14 @@ described as one: **Only this file's instance is closed, not the class.** Any head-resolved workflow holding 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 three - surviving routes: the injected `GITEA_TOKEN` (re-derived, since it posts with `creator: null`), - `RENOVATE_TOKEN` (a `write:repository` bot PAT in the same secret store, so it posts with a real - creator and IS inherited), and a collaborator's own token. So the "careless + 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 + `write:repository` bot PAT in the same secret store, so it posts with a real creator and IS + inherited, #742), a collaborator's own token, and the `v*` tag push — which matters less for + forging this status than for what else it does: `docker-build.yml` publishes `:prod` from a tagged + ref, and a tag may point at any commit, so it ships a prod image with no PR, review or status. + None of which is even required — direct pushes to `main` are server-side permitted, so the gate + can be skipped without forging anything (#743). So the "careless change rather than a hostile one" posture below still describes the repo accurately — it is simply no longer *this* workflow that is the weakest link. An untrusted-contributor repo would still need the classification moved somewhere no PR can diff --git a/scripts/tests/test_pr_changed_files.py b/scripts/tests/test_pr_changed_files.py index fefc61acd..b97bced4a 100644 --- a/scripts/tests/test_pr_changed_files.py +++ b/scripts/tests/test_pr_changed_files.py @@ -791,9 +791,13 @@ def test_no_OTHER_workflow_writes_the_review_verdict_status(): The trigger test above hardens ONE file, and by construction cannot see the more general problem: the gate is forgeable by *any* head-resolved workflow holding credentials that can POST a commit status, not only by the gate's own definition. `docker-build.yml` is exactly that — it - triggers on `pull_request` (correctly; it builds the PR's code) and carries `ETV_STATUS_AUTH`, - which a probe confirmed can WRITE statuses, not merely read them. That residual is tracked in - ersatztv#697 and is NOT closed by this test. + triggers on `pull_request` (correctly; it builds the PR's code) and carries `ETV_STATUS_AUTH`. + A probe once confirmed those credentials could WRITE statuses, not merely read them + (ersatztv#697); a follow-up probe confirmed `REGISTRY_PASSWORD` is now scoped off + `write:repository` and the same POST returns 403 (see `ci.actions-credential-scoping`). That + closes the registry-credential route, not the general problem — `GITEA_TOKEN`, `RENOVATE_TOKEN`, + and a collaborator's own token can all still POST a status, and none of that is closed by this + test. What this test does close is the cheap regression: a second workflow quietly starting to write the context. It is a guard against drift, not a security boundary — a workflow can still write @@ -1703,8 +1707,10 @@ def test_a_LEGACY_verdict_with_no_recorded_base_is_still_honoured(tmp_path): def test_an_APPENDED_base_cannot_override_the_real_one(tmp_path): - """The description is attacker-influencable by anyone who can POST a status (#697), so the parse - must not be trickable into reading a second, appended base. + """The description is attacker-influencable by anyone who can POST a status — the registry + credential's own route closed with #697, but `GITEA_TOKEN`, `RENOVATE_TOKEN`, and a + collaborator's own token still can — so the parse must not be trickable into reading a second, + appended base. This defeated the greedy `##` parse. The implementation no longer parses at all — it requires the description to END with the exact literal marker AND to contain exactly one marker — so a second