diff --git a/DECISIONS-LOG.md b/DECISIONS-LOG.md index deb0c8e..195c1dd 100644 --- a/DECISIONS-LOG.md +++ b/DECISIONS-LOG.md @@ -4924,3 +4924,43 @@ being unable to recur. 4. `required_approvals: 1` means a PR you open as `jakman85` needs `gitea-admin` to approve. If that friction bites, either open agent work as `gitea-admin` and review as `jakman85`, or drop back to relying on the status check alone. + +## 2026-09-29 (later still) — the enforcement we "turned on" was overridable by the account that merges + +`enable_status_check: true` and `required_approvals: 1` were set earlier today and recorded as +done. **They did not bite.** `block_admin_merge_override` was `false`, and Gitea lets repository +admins override merge requirements without it. `jakman85` holds `owner`, so both requirements +were bypassable by exactly the account that performs every merge. + +**The evidence was already on the record and nobody read it:** PRs #5, #6, #7, #8 and #9 all +merged while `required_approvals: 1` was in force, with **zero approvals recorded on any of +them**. A one-line check (`/pulls/{n}/reviews`) would have shown it at any point. + +Two process failures, both mine, both the same shape as the original defect: + +1. **I verified with the wrong view.** `/branches/main` reports `protected`, + `enable_status_check` and `required_approvals` but NOT `block_admin_merge_override`. It + showed everything green while the rule was overridable. The authoritative object is + `/branch_protections/main` — read it and enumerate its keys. +2. **I probed for a field name I assumed existed.** This Gitea (1.24.3) has no + `apply_to_admins`. Asking `jq` for it returns `null`, which reads exactly like "set to + false" but means "key absent" — so a PATCH that silently did nothing looked like a real + before/after. **`jq` returning null is not a value; it may be an absence.** Enumerate with + `to_entries`. + +Now `block_admin_merge_override: true`, applied and read back. `infra/gitea/require-gate.sh` +sets it, and `infra/gitea/block-admin-override.sh` fixes it in isolation. + +**This is still only a setting.** It is proven when a green PR is REFUSED a merge until +approved — and that has not been observed yet. This PR is the test: if it merges without an +approval, the fix did not work and the finding stands. + +### The governance question this exposes + +There are two accounts: `jakman85` (owner, the human) and `gitea-admin` (the agent). With +approvals now genuinely required, an agent-authored PR is approved by the human — which is the +intent. But a PR authored by `jakman85` needs `gitea-admin` to approve it, i.e. the automation +account rubber-stamping the human. That is ceremony, not review. Options: keep it and accept +the ceremony; drop `required_approvals` to 0 and rely on the status check, which is the gate +that actually verifies something; or add a second human account. **This is a decision for the +operator, not a default to be chosen silently.** diff --git a/infra/gitea/README.md b/infra/gitea/README.md index 345c741..719e0f6 100644 --- a/infra/gitea/README.md +++ b/infra/gitea/README.md @@ -55,8 +55,27 @@ All four steps are complete and each was verified, not assumed: | 4. Collaborator + approvals | ✅ `jakman85` is owner; `required_approvals: 1` | Direct push to `main` was **provoked and rejected** (`pre-receive hook declined`) as -`gitea-admin`, a site admin — so protection is not merely declared, it holds for the account -agents authenticate as. +`gitea-admin`, a site admin — so push protection is not merely declared, it holds for the +account agents authenticate as. + +### 5. Block the admin override — the step the first pass MISSED + +`enable_status_check` and `required_approvals` were set on 2026-09-29 and **still did not +bite**, because `block_admin_merge_override` was `false`. Gitea lets repo admins override +merge requirements without it, and `jakman85` holds `owner` — so the two settings that looked +like the gate were overridable by precisely the account that merges. + +**The evidence was sitting in plain sight:** PRs #5 through #9 all merged with +`required_approvals: 1` in force and **zero approvals recorded on any of them**. Checking the +*branch* view (`/branches/main`) hid this — it does not expose the field. Read the rule object +(`/branch_protections/main`) and enumerate its keys. + +> **Do not probe for field names you assume exist.** This Gitea (1.24.3) has no +> `apply_to_admins`; asking `jq` for it returns `null`, which reads like "set to false" but +> means "key absent". Enumerate with `to_entries` and act on what is really there. + +Now `block_admin_merge_override: true`. **That is still only a setting, not evidence** — it is +proven when a green PR is REFUSED a merge until approved, and not before. ### `require-gate.sh` / `verify-then-enforce.sh` diff --git a/infra/gitea/block-admin-override.sh b/infra/gitea/block-admin-override.sh new file mode 100755 index 0000000..ea3b34e --- /dev/null +++ b/infra/gitea/block-admin-override.sh @@ -0,0 +1,36 @@ +#!/usr/bin/env bash +# Stop repository admins from overriding the merge requirements on `main`. +# +# WHY. Gitea lets repo admins override branch-protection merge requirements unless +# `block_admin_merge_override` is set. It was FALSE, and jakman85 holds `owner` -- so +# required_approvals and the required status check were both overridable by the one account +# that actually merges. Evidence: PRs #5-#9 all merged with ZERO approvals recorded against +# `required_approvals: 1`. +# +# NOTE ON FIELD NAMES: this Gitea (1.24.3) has NO `apply_to_admins` field. Querying it with jq +# returns null, which reads like "set to false" but actually means "key absent". Enumerate the +# real keys (`to_entries`) rather than probing names you assume exist. +# +# Recoverable: an owner can PATCH this back to false if it gets in the way. +set -euo pipefail +HOST=gitea.home.keithjackson.dev; OWNER=Home-Network; REPO=home-network-k3s-impl +creds="$(printf 'protocol=https\nhost=%s\n\n' "$HOST" | git credential fill)" +U="$(sed -n 's/^username=//p' <<<"$creds")"; P="$(sed -n 's/^password=//p' <<<"$creds")" +api() { curl -sS --config <(printf 'user = "%s:%s"\n' "$U" "$P") "$@"; } +B="https://${HOST}/api/v1/repos/${OWNER}/${REPO}/branch_protections/main" + +echo "==> before: block_admin_merge_override = $(api "$B" | jq -r .block_admin_merge_override)" +api -X PATCH -H 'Content-Type: application/json' \ + -d '{"block_admin_merge_override": true}' "$B" >/dev/null +AFTER=$(api "$B" | jq -r .block_admin_merge_override) +echo "==> after: block_admin_merge_override = $AFTER" +[ "$AFTER" = true ] || { echo "PATCH did not take -- do not record this as enforced." >&2; exit 1; } + +api "$B" | jq -r '{enable_push,enable_status_check,status_check_contexts,required_approvals,block_admin_merge_override}' +cat <<'NOTE' + +NOW PROVE IT -- the setting is not the evidence. On the next PR: + * let the gate go green, then try to merge WITHOUT approving it. + * Gitea must REFUSE. If it merges, this still is not doing its job. +Do not record approvals as enforced until you have seen a merge refused. +NOTE diff --git a/infra/gitea/require-gate.sh b/infra/gitea/require-gate.sh index 7490e15..4eb21f5 100755 --- a/infra/gitea/require-gate.sh +++ b/infra/gitea/require-gate.sh @@ -12,6 +12,15 @@ # * NOT `gate / conformance (*)` -- that job is `if: false`, and a skipped Gitea job reports # SUCCESS ("Has been skipped"). Requiring it would pass VACUOUSLY forever while nothing # ran, which under HARD RULE 4 is worse than a deadlock: a deadlock is loud. +# block_admin_merge_override: true is LOAD-BEARING and was missing on the first pass. Gitea +# lets repo admins override merge requirements without it, so required_approvals and the +# required status check are both bypassable by exactly the account that does the merging. +# Measured: PRs #5-#9 merged with ZERO approvals recorded against required_approvals: 1. +# +# FIELD NAMES: this Gitea (1.24.3) has NO `apply_to_admins`. Asking jq for it returns null, +# which reads like "false" but means "key absent" -- enumerate real keys with `to_entries` +# instead of probing names you assume exist. That mistake cost a wasted PATCH. +# # * NOT the `(push)` variant -- after PR #6 there are no per-branch push runs at all, so a # PR would never produce that context. set -euo pipefail @@ -28,7 +37,8 @@ jq -n '{ required_approvals: 1, block_on_rejected_reviews: true, dismiss_stale_approvals: true, - block_on_outdated_branch: false + block_on_outdated_branch: false, + block_admin_merge_override: true }' | api -X PATCH -H 'Content-Type: application/json' --data @- \ "https://${HOST}/api/v1/repos/${OWNER}/${REPO}/branch_protections/main" >/dev/null