timothy
b2a5c72bfe
Cross-family review established the residual is materially larger than the previous commit said, and that saying "docker-build.yml / ETV_STATUS_AUTH" understates it. Gitea injects `GITEA_TOKEN` into EVERY job automatically, defaulting to read/write. So the set of workflows that can POST `review-verdict/h10` is not a short list to audit -- it is all of them, plus `workflow_dispatch` (1.24+ loads the definition from the selected branch) and `push`-triggered ones. A collaborator's own write-scoped API token is a route with no workflow at all, because branch protection binds the required CONTEXT, not its issuer. Recorded in #697. The same review found three defects in the exemption path itself, none of them introduced here and none closed here: a retarget race that enumerates a docs-only diff against a scratch base while the enumerator revalidates only head.sha, a Renovate-PR hijack (the exemption reads the immutable PR creator, not who pushed the head), and an inherited `success` that short-circuits before any PR/base/author/file check. Filed together as #698, since they share one root cause -- the gate trusts state it cannot attribute. Also drops the claim that the three properties are "pinned by tests". The tests pin the workflow's SHAPE; no in-repository test can establish status-authority isolation, and the sibling-workflow guard added in the previous commit catches only a workflow that names the context in plain text. Trimmed to the 60-line prose ceiling for the third time in this session. That is ersatztv#688 -- the ceiling is now deleting rationale two reviewers asked for. Refs: #672 Refs: #697 Refs: #698 Decisions-Edit: yes
7.2 KiB
key, title, status, since, supersedes, superseded-by, rule, signals, mechanics
| key | title | status | since | supersedes | superseded-by | rule | signals | mechanics |
|---|---|---|---|---|---|---|---|---|
| ci.gate-trigger-base-resolved | 2026-07-28 — `review-verdict.yml` triggers on `pull_request_target` scoped to `branches: [main]`, so the PR under judgment cannot supply the gate's own definition (#672) | active | 2026-07-28 | none | none | 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`. Tracked in #697; the exemption path has its own separate defects in #698. | 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, 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 | `on: pull_request_target: {branches: [main], types: [opened, reopened, synchronize, ready_for_review]}`; 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 |
ci.shared-pr-file-enumeration had this job check out the PR's base ref so the PR cannot supply
the scripts that judge it — real but partial, as that record said: it does not bind the job
definition. This closes that half.
What was actually wrong. Gitea, like GitHub, resolves a pull_request workflow definition from
the PR's own head, so a PR editing review-verdict.yml ran its own rewritten copy — which could
delete the base checkout or skip straight to posting review-verdict/h10=success for its head sha.
Two things that look preventive were not: PROTECTED is defined by the same rewritten file, and
branch protection requires the context, not an author, while carrying required_approvals: 0.
Measured, not inferred. The premise is a claim about someone else's software, so it was settled
on this instance (Gitea 1.25.4) with four scratch PRs, not by analogy to GitHub. A PR rewriting
the workflow posted its own probe context and the real review-verdict/h10 was never written — the
base definition never ran. Under pull_request_target the identical rewrite was ignored: the base
definition ran and posted h10=pending, on opened and synchronize alike, with secrets still
available. With branches: [main], a PR into a non-main base produced no run and no status at all.
The fourth probe is the negative one that established the residual below; it is counted because
omitting it is what turns an honest partial into an overclaim. The probes posted only probe-named
contexts, never a forged h10. Not measured: whether reopened/ready_for_review fire here.
Why branches: [main] is load-bearing rather than tidy. Base resolution means the base branch
supplies the definition, and anyone who can push a branch can make it a base — so dropping the filter
trades a head-supplied gate for a base-supplied one and closes nothing. Worse than lateral: a commit
status is repo-global per sha (#663), so a success forged against a scratch base is inherited
by a later genuine PR into main with the same head. The filter keeps "the base" and "main" the
same thing.
Why pull_request_target is not the footgun it usually is. Its standard danger is executing
untrusted head code with a privileged token; this job executes none, checking out base.sha with
persist-credentials: false and running only that tree's scripts. Trigger and checkout are one
decision — under this trigger a head checkout would be strictly worse than #672 was.
Options not taken. required_approvals: 1 was the cheapest mechanical fix and is unusable here:
Gitea forbids approving your own PR and this is effectively a single-maintainer repo, so it would
deadlock every PR rather than gate the dangerous ones. Verifying the status author needs an actor
the PR cannot control, and the tampered workflow holds the same GITEA_TOKEN.
Severity, stated plainly. Never remotely exploitable — pushing a branch requires write access, so the threat model is a compromised contributor, who has other paths. Fixed because a gate whose authority the judged thing can assert is not a gate, not because an attack was expected.
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
route with no workflow at all — branch protection binds the context, not its issuer. Full
inventory in #697; the exemption path's own defects are #698. No in-repository test can establish
status-authority isolation: the sibling-workflow guard added here catches only plain-text naming of
the context. Spelled out because the predecessor record's documented failure was a reassuring
sentence exactly where a catalog reader stops.
Residual. The job's own context is renamed to ... (pull_request_target), safe only because it
was never one of branch protection's required contexts (the two docker-build.yml contexts plus
review-verdict/h10); adding it would let the workflow satisfy the gate by merely running. A trap
for #697: those two carry the literal suffix (pull_request), so giving docker-build.yml the
same treatment renames them and deadlocks every merge unless branch protection is edited in the same
operation. A PR based off a non-main branch now gets no status where it previously got one —
fail-closed, and it removes a #663 inheritance hazard — but edited is absent from types:, so a
PR retargeted onto main stays statusless until its next synchronize.