diff --git a/CLAUDE.md b/CLAUDE.md index f50b701..4f707aa 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -36,6 +36,28 @@ sensitive path, so the policy cannot be widened or narrowed without a human. That stage is **fail-closed** — if it cannot evaluate, it blocks. Unlike `gitea-settings`, where a missing credential is a lock-out, here a skip would be a bypass. + +**The administrator override (2026-10-01).** The rule above was unsatisfiable for the one human +who runs this repo: Gitea refuses to let an author approve their own PR, and `jakman85` is the +only collaborator — so a sensitive-path PR the operator opened could never be merged, *including +the PR that would fix that.* An administrator may therefore waive the review by commenting on +the PR: + + /approve-override + +The directive must **start a line**, the **reason is mandatory**, the commenter's `admin`/`owner` +permission is re-checked **live** when the gate runs, and the comment must be **newer than the +head commit** — push again and you must override again. The overrider, the timestamp and the +reason are printed in the gate log and the job summary. For an administrator, "a change to the +gate cannot be gated by the gate" is therefore **advisory, not structural**; what is kept is an +attributed record of each waiver. Note that the agent account `gitea-admin` also holds `owner` +here, so an agent *can* clear this on its own PR — **do not**: posting that directive to unblock +your own agent work is exactly the self-approval the stage exists to prevent. Get the operator +to post it, or move the change out of the sensitive paths. + +**`required_approvals` on `main` is `0`**, not 1 — the gate is the approver. That drops the +approval *count* only: `block_admin_merge_override` and `enable_status_check` both remain `true`, +so **a red gate is unmergeable by everyone, administrators included.** - **Do not `kubectl apply` / `edit` / `patch` a durable change.** Flux reconciles `main`; anything applied by hand is drift that the next reconcile reverts or, worse, adopts. @@ -53,12 +75,19 @@ Verified against the live Gitea on 2026-09-29: | `main` protected, direct push rejected | ✅ `protected: true`, `user_can_push: false` — **verified by a real rejected push**, not by the API's word | | PR blocked until the gate passes | ✅ `enable_status_check: true`, context `gate / gate (pull_request)` | | Gate runs on every PR | ✅ runner `incluster-dind` online; ~1m35s | -| Approval required before merge | ✅ `required_approvals: 1` | +| Gate cannot be overridden, by anyone | ✅ `block_admin_merge_override: true` | +| Human approval required before merge | ⬜ **No — `required_approvals: 0` since 2026-10-01.** The gate is the approver; sensitive paths are gated by `ci/approval-policy.sh` instead. See §1 | -All four were FALSE as recently as 2026-09-29, while this repo's own log asserted they were +The first three were FALSE as recently as 2026-09-29, while this repo's own log asserted they were true. They are true now because each was measured, and the push rejection was provoked rather than assumed. **Re-measure before relying on any of them** — see §2b. +The fourth row deliberately changed on 2026-10-01. `required_approvals: 1` was a **lock-out**, not +a protection: with a single collaborator and Gitea refusing author self-approval, nothing could +satisfy it (measured on PR #35 — green gate, zero reviews, unmergeable). It also contradicted the +model already written into `ci/approval-policy.sh` and `ci/gate.sh`. Rationale and the live-change +script: `infra/gitea/allow-admin-self-merge.sh`. + Still not covered: `vars.CLUSTER_AVAILABLE` is unset, so **L2/L3 conformance does not run in CI**. A green PR therefore proves the L0/L1 offline subset, not runtime behaviour. diff --git a/ci/approval-override.py b/ci/approval-override.py new file mode 100755 index 0000000..913ccfd --- /dev/null +++ b/ci/approval-override.py @@ -0,0 +1,139 @@ +#!/usr/bin/env python3 +"""ci/approval-override.py — select the admin-override comment on a PR, if there is a valid one. + +This script decides NOTHING about authority. It parses a PR's comment list and returns the +single override *candidate* that ci/approval-policy.sh should then try to authorise against +the live Gitea permission API. Keeping the two apart is the point: + + * parsing is pure, offline, and therefore TESTABLE (ci/tests/approval-override.test.sh); + * authorisation needs the API and must stay in the stage that fails closed. + +A candidate is a comment whose body contains, AT THE START OF A LINE, the directive followed +by a non-empty reason: + + /approve-override the gate cannot gate itself and I am the only human reviewer + +WHY EACH RULE IS HERE + + * START OF A LINE, not "anywhere in the body". Otherwise quoting someone else's override + ("you could just /approve-override this") re-arms it. Markdown quotes are stripped of a + leading '>' before matching for the same reason -- a quoted directive must NOT count, so + lines beginning with '>' are DROPPED, not unquoted. + * A REASON IS MANDATORY. A bare directive records that a review was waived but not why, + which is the difference between an audit trail and a rubber stamp. + * STALE OVERRIDES ARE DISCARDED. An override earned at commit A must not silently cover + commit B. Any candidate created at or before the head commit's timestamp is dropped -- + the same semantics Gitea's own `dismiss_stale_approvals` applies to reviews. Push new + commits and you must re-override. + * THE LATEST CANDIDATE WINS, so a second override comment supersedes the first rather than + both being live. + +Output: one TSV line (login, created_at, reason) on stdout, or nothing at all when there is no +valid candidate. Never exits non-zero for "no override" -- absence is a normal answer, and the +caller must not be able to confuse it with a parse crash. + +Input, all via env so nothing lands in `ps`: + COMMENTS_JSON the Gitea /issues/{n}/comments array + HEAD_TIME ISO-8601 timestamp of the PR head commit + DIRECTIVE the override directive, e.g. /approve-override +""" +import json +import os +import re +import sys +from datetime import datetime, timezone + + +def parse_ts(value): + """Gitea emits RFC3339. Return an aware datetime, or None if it is unparseable. + + Unparseable is NOT 'treat as fresh': the caller drops such a candidate, because a + timestamp we cannot read is a staleness check we cannot perform. + """ + if not value: + return None + text = str(value).strip() + # Python < 3.11 cannot read a trailing 'Z' in fromisoformat. + if text.endswith(("Z", "z")): + text = text[:-1] + "+00:00" + try: + parsed = datetime.fromisoformat(text) + except ValueError: + return None + if parsed.tzinfo is None: + parsed = parsed.replace(tzinfo=timezone.utc) + return parsed + + +def extract_reason(body, directive): + """Return the reason from the first line that STARTS with the directive, else None.""" + pattern = re.compile( + r"^[ \t]*" + re.escape(directive) + r"(?=[ \t:]|$)[ \t:]*(.*)$" + ) + for raw in (body or "").splitlines(): + # A quoted line is somebody repeating an override, not casting one. + if raw.lstrip().startswith(">"): + continue + match = pattern.match(raw) + if match: + reason = match.group(1).strip() + return reason or None + return None + + +def main(): + directive = os.environ.get("DIRECTIVE", "/approve-override").strip() + head_time = parse_ts(os.environ.get("HEAD_TIME", "")) + + try: + comments = json.loads(os.environ.get("COMMENTS_JSON") or "[]") + except (ValueError, TypeError): + # The caller validated this JSON already; if it is broken here, say so loudly on + # stderr and emit no candidate. Silence on stdout means "no override". + print("approval-override: COMMENTS_JSON is not valid JSON", file=sys.stderr) + return 0 + if not isinstance(comments, list): + print("approval-override: COMMENTS_JSON is not a list", file=sys.stderr) + return 0 + + candidates = [] + for comment in comments: + if not isinstance(comment, dict): + continue + login = ((comment.get("user") or {}).get("login") or "").strip() + reason = extract_reason(comment.get("body"), directive) + if not login or not reason: + continue + created_raw = comment.get("created_at") or comment.get("updated_at") or "" + created = parse_ts(created_raw) + if created is None: + print( + "approval-override: dropping override by '%s' — unreadable timestamp %r" + % (login, created_raw), + file=sys.stderr, + ) + continue + # An override must post-date the commit it covers. + if head_time is not None and created <= head_time: + print( + "approval-override: dropping STALE override by '%s' (%s) — not newer than " + "head commit (%s). Re-comment to override the current commit." + % (login, created.isoformat(), head_time.isoformat()), + file=sys.stderr, + ) + continue + candidates.append((created, login, reason, created_raw)) + + if not candidates: + return 0 + + # Latest wins: a later override supersedes an earlier one. + created, login, reason, created_raw = max(candidates, key=lambda c: c[0]) + # Keep the reason on one line — this becomes a TSV field. + reason = " ".join(reason.split()) + print("%s\t%s\t%s" % (login, created_raw, reason)) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/ci/approval-policy.sh b/ci/approval-policy.sh index 8b1d978..f541871 100755 --- a/ci/approval-policy.sh +++ b/ci/approval-policy.sh @@ -25,10 +25,45 @@ # true: if this cannot evaluate, an unreviewed change to the gate itself could merge. A skip is a # BYPASS, not a gap. So: cannot determine -> BLOCK. # +# THE ADMIN OVERRIDE (added 2026-10-01), and the lock-out it clears. +# +# The rule above was UNSATISFIABLE for the one human who runs this repo. Gitea refuses to let a +# pull request's author approve their own PR, and `jakman85` is the only collaborator — so "an +# APPROVED review from someone who is not its author" is a condition the sole administrator can +# never meet. Every sensitive-path PR they opened was permanently unmergeable, INCLUDING the PR +# that would fix it. Measured 2026-10-01: PR #35, gate green in 1m41s, zero reviews, blocked. +# +# So an administrator may waive the review by COMMENTING on the PR: +# +# /approve-override +# +# Authority comes from the LIVE Gitea permission API, never from the comment — the commenter must +# hold a permission listed in `.override.authorized_permissions` (admin/owner) at the moment the +# gate runs, so an old comment from a since-demoted account is worth nothing. The directive must +# start a line, a reason is mandatory, and the comment must POST-DATE the head commit: an override +# earned at commit A does not cover commit B. Choosing the candidate comment is the job of the +# pure, offline, unit-tested ci/approval-override.py; AUTHORISING it is the job of this stage, +# which fails closed. +# +# WHAT THIS COSTS, STATED PLAINLY. For an administrator, "a change to the gate cannot be gated by +# the gate" stops being structural and becomes advisory. That is the unavoidable consequence of +# there being one human; the alternative is a repository its owner cannot change. What is bought +# back is a RECORD — the waiver is an explicit act, attributed to an account, carrying a stated +# reason, in the PR history and in this stage's output. A silent bypass keyed on "is the author an +# admin?" would have been less code and is exactly the fail-open shape this repo keeps finding. +# +# AND THE PART THAT IS NOT COMFORTABLE. Authorisation is by permission ALONE, with no allowlist of +# human accounts (operator decision, 2026-10-01, taken with this consequence on the table). The +# AGENT account `gitea-admin` also reports permission `owner` here, so an agent can clear the +# sensitive-path review on its own PR by posting the directive — and agents author most PRs in +# this repo, so that is the common case, not an edge case. To narrow it without touching this +# logic, add an `override_admins` list to infra/gitea/approval-policy.json: the check below +# already requires membership whenever that key is present. +# # EXIT CODES -# 0 no sensitive paths touched (gate alone is sufficient), OR sensitive paths touched and a -# non-author APPROVED review is present -# 1 sensitive paths touched without such a review, OR the policy could not be evaluated +# 0 no sensitive paths touched (gate alone is sufficient), OR sensitive paths touched and +# either a non-author APPROVED review or a valid admin override is present +# 1 sensitive paths touched with neither, OR the policy could not be evaluated set -euo pipefail CI_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" REPO_ROOT="$(cd "${CI_DIR}/.." && pwd)" @@ -65,7 +100,11 @@ api() { curl -sS --max-time 20 -H "Authorization: token ${TOKEN}" "$@"; } FILES_JSON="$(api "${GITEA_URL}/api/v1/repos/${GITEA_REPO}/pulls/${PR}/files?limit=1000" || true)" PR_JSON="$(api "${GITEA_URL}/api/v1/repos/${GITEA_REPO}/pulls/${PR}" || true)" REVIEWS_JSON="$(api "${GITEA_URL}/api/v1/repos/${GITEA_REPO}/pulls/${PR}/reviews" || true)" -for v in FILES_JSON PR_JSON REVIEWS_JSON; do +# PR conversation comments live under the ISSUES endpoint. `/pulls/{n}/comments` returns review +# (diff) comments ONLY, so an override posted in the normal comment box would never appear in it. +# Verified against this Gitea (1.24.3) rather than assumed. +COMMENTS_JSON="$(api "${GITEA_URL}/api/v1/repos/${GITEA_REPO}/issues/${PR}/comments?limit=1000" || true)" +for v in FILES_JSON PR_JSON REVIEWS_JSON COMMENTS_JSON; do jq -e . >/dev/null 2>&1 <<<"${!v}" || { err "could not read ${v} from Gitea for PR #${PR} — BLOCKING rather than assuming." summary "- approval-policy: **BLOCKED — Gitea unreadable**" @@ -107,11 +146,103 @@ if [ -n "$APPROVER" ]; then exit 0 fi +# ───────────────────────────────────────────────────────────────────────────────────────── +# No non-author approval exists. Before refusing, look for an administrator's explicit +# override. See the header for why this hatch exists and what it costs. +# ───────────────────────────────────────────────────────────────────────────────────────── +OVERRIDE_ENABLED="$(jq -r '.override.enabled // false' "$DECL")" +DIRECTIVE="$(jq -r '.override.comment_directive // "/approve-override"' "$DECL")" +HEAD_SHA="$(jq -r '.head.sha // empty' <<<"$PR_JSON")" + +if [ "$OVERRIDE_ENABLED" = "true" ]; then + # The head commit's timestamp anchors staleness. Read it from GITEA, not from the local + # tree: CI checks out a merge commit, whose date is not the head commit's date. + HEAD_TIME="" + if [ -n "$HEAD_SHA" ]; then + HEAD_COMMIT_JSON="$(api "${GITEA_URL}/api/v1/repos/${GITEA_REPO}/git/commits/${HEAD_SHA}" || true)" + HEAD_TIME="$(jq -r '.commit.committer.date // .created // empty' <<<"${HEAD_COMMIT_JSON:-null}" 2>/dev/null || true)" + fi + if [ -z "$HEAD_TIME" ]; then + err "could not read the head commit's timestamp for PR #${PR} — BLOCKING." + err " Without it an override cannot be aged, and an unaged override is one that silently" + err " covers commits nobody waived review on." + summary "- approval-policy: **BLOCKED — head commit timestamp unreadable** (cannot age an override)" + exit 1 + fi + + # Pure, offline, unit-tested selection: which comment, if any, is a live override? + OVERRIDE_TSV="$(COMMENTS_JSON="$COMMENTS_JSON" HEAD_TIME="$HEAD_TIME" DIRECTIVE="$DIRECTIVE" \ + python3 "${CI_DIR}/approval-override.py")" || { + err "ci/approval-override.py failed — BLOCKING rather than assuming there is no override." + summary "- approval-policy: **BLOCKED — override parser failed**" + exit 1; } + + if [ -n "$OVERRIDE_TSV" ]; then + OV_USER="$(cut -f1 <<<"$OVERRIDE_TSV")" + OV_WHEN="$(cut -f2 <<<"$OVERRIDE_TSV")" + OV_WHY="$(cut -f3- <<<"$OVERRIDE_TSV")" + + # AUTHORITY IS MEASURED LIVE, never inferred from the comment or from a name in git. + PERM_JSON="$(api "${GITEA_URL}/api/v1/repos/${GITEA_REPO}/collaborators/${OV_USER}/permission" || true)" + OV_PERM="$(jq -r '.permission // empty' <<<"${PERM_JSON:-null}" 2>/dev/null || true)" + if [ -z "$OV_PERM" ]; then + err "'${OV_USER}' posted ${DIRECTIVE}, but their permission on ${GITEA_REPO} could not be" + err " read — BLOCKING. An override whose authority is unverified is not an override." + summary "- approval-policy: **BLOCKED — cannot verify \`${OV_USER}\`'s permission**" + exit 1 + fi + + # Authorised when the LIVE permission is one of the declared ones and — only if an + # `override_admins` allowlist exists at all — the account is on it. No allowlist means + # permission alone, which is the operator's standing decision; adding the key narrows + # this with no change to any code. + AUTHORISED="$(DECL="$DECL" OV_PERM="$OV_PERM" OV_USER="$OV_USER" python3 -c ' +import json, os +d = json.load(open(os.environ["DECL"])).get("override", {}) +perms = d.get("authorized_permissions") or ["admin", "owner"] +allow = d.get("override_admins") +ok_perm = os.environ["OV_PERM"] in perms +ok_list = True if allow is None else os.environ["OV_USER"] in allow +print("yes" if (ok_perm and ok_list) else "no") +')" + + if [ "$AUTHORISED" = "yes" ]; then + if [ "$OV_USER" = "$AUTHOR" ]; then + warn "ADMIN OVERRIDE — SELF-WAIVED. '${OV_USER}' authored this PR and waived the" + warn " non-author review requirement on their own change. Permitted (permission" + warn " '${OV_PERM}'), deliberate, and shouted about here precisely because it is not free." + else + warn "ADMIN OVERRIDE by '${OV_USER}' (permission '${OV_PERM}'); author is '${AUTHOR}'." + fi + warn " when: ${OV_WHEN} (head commit ${HEAD_SHA:0:8}, ${HEAD_TIME})" + warn " reason: ${OV_WHY}" + warn " review waived on: $(printf '%s ' $SENSITIVE)" + ok "sensitive paths cleared by administrator override — this stage does not block." + SELFNOTE=""; [ "$OV_USER" = "$AUTHOR" ] && SELFNOTE=", **self-waived as author**" + summary "- approval-policy: **OVERRIDDEN by \`${OV_USER}\`** (permission \`${OV_PERM}\`${SELFNOTE})" + summary " - reason: ${OV_WHY}" + summary " - review waived on: $(printf '\`%s\` ' $SENSITIVE)" + exit 0 + fi + + err "'${OV_USER}' posted ${DIRECTIVE} but is NOT authorised to override: live permission is" + err " '${OV_PERM}'. The authorised set is declared in infra/gitea/approval-policy.json." + summary "- approval-policy: **BLOCKED — \`${OV_USER}\` may not override** (permission \`${OV_PERM}\`)" + exit 1 + fi +fi + err "This PR changes the pipeline or the security posture it rests on, and has NO approving" err " review from anyone other than its author ('${AUTHOR}')." err " A change to the gate cannot be gated by the gate — that is the one thing CI cannot" err " honestly approve for itself. Get a human review, or move the change out of:" printf ' %s\n' $SENSITIVE >&2 err " The list lives in infra/gitea/approval-policy.json, with a reason per entry." -summary "- approval-policy: **BLOCKED — sensitive paths need a non-author approval**" +if [ "$OVERRIDE_ENABLED" = "true" ]; then + err " Or, if you hold admin/owner on this repository, comment on the PR:" + err " ${DIRECTIVE} " + err " The reason is mandatory, the directive must START a line, and the comment must be" + err " NEWER than head commit ${HEAD_SHA:0:8} — push again and you must override again." +fi +summary "- approval-policy: **BLOCKED — needs a non-author approval, or an admin \`${DIRECTIVE}\` comment**" exit 1 diff --git a/ci/tests/approval-override.test.sh b/ci/tests/approval-override.test.sh new file mode 100755 index 0000000..d792d4b --- /dev/null +++ b/ci/tests/approval-override.test.sh @@ -0,0 +1,145 @@ +#!/usr/bin/env bash +# ci/tests/approval-override.test.sh — proves ci/approval-override.py selects the override it +# claims to select, and REJECTS the ones it claims to reject. +# +# This parser decides whether a human review requirement gets waived. Every rule in it exists +# because the obvious implementation ("does the body contain the directive?") is wrong in a way +# that is invisible until abused: a quoted directive would re-arm an override, a bare directive +# would waive review with no stated reason, and an override posted against commit A would go on +# covering commit B forever. A guard is worth nothing until somebody has watched it refuse, which +# is the same reasoning as ci/tests/decisions-log.test.sh one layer up. +# +# The parser is pure and offline BY DESIGN — authorisation (the live permission lookup) lives in +# ci/approval-policy.sh, which cannot be tested without Gitea. That split is what makes this file +# possible, and it is also why a green run here does NOT prove the stage authorises correctly. +set -uo pipefail +CI_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +SUT="${CI_DIR}/approval-override.py" + +PASS=0 FAIL=0 +ok() { printf ' ok %s\n' "$*"; PASS=$((PASS+1)); } +bad() { printf ' FAIL %s\n' "$*"; FAIL=$((FAIL+1)); } + +HEAD_TIME="2026-10-01T12:00:00Z" +AFTER="2026-10-01T12:30:00Z" +BEFORE="2026-10-01T11:30:00Z" + +# run -> stdout of the parser (stderr is diagnostic only) +run() { + COMMENTS_JSON="$1" HEAD_TIME="${2:-$HEAD_TIME}" DIRECTIVE="${3:-/approve-override}" \ + python3 "$SUT" 2>/dev/null +} + +# comment +comment() { + python3 -c ' +import json, sys +print(json.dumps([{"user": {"login": sys.argv[1]}, "created_at": sys.argv[2], "body": sys.argv[3]}])) +' "$1" "$2" "$3" +} + +expect_none() { + local label="$1" out + out="$(run "$2")" + if [ -z "$out" ]; then ok "$label"; else bad "$label (got: ${out})"; fi +} + +expect_user() { + local label="$1" json="$2" want="$3" out got + out="$(run "$json")" + got="$(cut -f1 <<<"$out")" + if [ "$got" = "$want" ]; then ok "$label"; else bad "$label (wanted ${want}, got '${got}')"; fi +} + +expect_reason() { + local label="$1" json="$2" want="$3" out got + out="$(run "$json")" + got="$(cut -f3- <<<"$out")" + if [ "$got" = "$want" ]; then ok "$label"; else bad "$label (wanted '${want}', got '${got}')"; fi +} + +echo "== ci/approval-override.py" + +# --- the happy path ------------------------------------------------------------------- +expect_user "a directive with a reason, posted after the head commit, is selected" \ + "$(comment jakman85 "$AFTER" '/approve-override sole admin, gate is green')" jakman85 + +expect_reason "the reason is returned verbatim" \ + "$(comment jakman85 "$AFTER" '/approve-override sole admin, gate is green')" \ + 'sole admin, gate is green' + +# --- absence is a normal answer, not an error ----------------------------------------- +expect_none "no comments at all" '[]' +expect_none "an ordinary comment" "$(comment jakman85 "$AFTER" 'looks good to me')" + +# --- a reason is MANDATORY ------------------------------------------------------------ +expect_none "a bare directive (no reason) is refused" \ + "$(comment jakman85 "$AFTER" '/approve-override')" +expect_none "a directive followed by only whitespace is refused" \ + "$(comment jakman85 "$AFTER" '/approve-override ')" + +# --- the directive must START a line -------------------------------------------------- +expect_none "the directive mid-sentence does not count" \ + "$(comment jakman85 "$AFTER" 'you could just /approve-override this one')" +expect_none "a QUOTED directive does not re-arm an override" \ + "$(comment jakman85 "$AFTER" '> /approve-override quoting someone else')" +expect_user "leading whitespace before the directive is fine" \ + "$(comment jakman85 "$AFTER" ' /approve-override indented but mine')" jakman85 +expect_user "the directive on a later line of a longer comment counts" \ + "$(comment jakman85 "$AFTER" 'some preamble +/approve-override the real reason here')" jakman85 + +# --- staleness: an override must post-date the commit it waives review on ------------- +expect_none "an override OLDER than the head commit is dropped as stale" \ + "$(comment jakman85 "$BEFORE" '/approve-override stale, predates the push')" +expect_none "an override exactly AT the head commit time is dropped (strictly newer)" \ + "$(comment jakman85 "$HEAD_TIME" '/approve-override same instant')" + +# --- malformed input is never a free pass -------------------------------------------- +expect_none "an unreadable timestamp drops the candidate" \ + "$(comment jakman85 'not-a-date' '/approve-override bad clock')" +expect_none "a comment with no author is dropped" \ + '[{"user": {}, "created_at": "2026-10-01T12:30:00Z", "body": "/approve-override no login"}]' +expect_none "invalid JSON yields no override (and never a traceback into stdout)" \ + 'this is not json' +expect_none "a JSON object instead of a list yields no override" \ + '{"user": "nope"}' + +# --- latest wins ---------------------------------------------------------------------- +TWO='[ + {"user": {"login": "early"}, "created_at": "2026-10-01T12:10:00Z", "body": "/approve-override first"}, + {"user": {"login": "latest"}, "created_at": "2026-10-01T12:40:00Z", "body": "/approve-override second"} +]' +expect_user "the LATEST override supersedes an earlier one" "$TWO" latest +expect_reason "and brings its own reason" "$TWO" second + +# --- offsets, not just Z -------------------------------------------------------------- +# Gitea emits RFC3339 WITH AN OFFSET (measured: 2026-10-01T23:09:08-05:00), not a trailing Z. +# A parser that only handles 'Z' silently drops every real override, so this case is load-bearing. +expect_user "an RFC3339 offset timestamp (-05:00), as Gitea actually emits, parses" \ + '[{"user": {"login": "jakman85"}, "created_at": "2026-10-01T08:30:00-05:00", "body": "/approve-override offset form"}]' \ + jakman85 +expect_none "and an offset timestamp that is genuinely earlier is still stale" \ + '[{"user": {"login": "jakman85"}, "created_at": "2026-10-01T06:00:00-05:00", "body": "/approve-override offset, stale"}]' + +# --- the directive is configurable ---------------------------------------------------- +out="$(run "$(comment jakman85 "$AFTER" '/admin-waive because policy says so')" "$HEAD_TIME" '/admin-waive')" +if [ "$(cut -f1 <<<"$out")" = "jakman85" ]; then + ok "a custom directive from the declaration is honoured" +else + bad "a custom directive from the declaration is honoured (got '${out}')" +fi +expect_none "the default directive does not match a custom-directive comment" \ + "$(comment jakman85 "$AFTER" '/admin-waive because policy says so')" + +# --- no staleness anchor: do not invent one ------------------------------------------- +out="$(COMMENTS_JSON="$(comment jakman85 "$BEFORE" '/approve-override no anchor')" \ + HEAD_TIME="" DIRECTIVE=/approve-override python3 "$SUT" 2>/dev/null)" +if [ "$(cut -f1 <<<"$out")" = "jakman85" ]; then + ok "with no HEAD_TIME the staleness test is skipped, not faked (caller blocks instead)" +else + bad "with no HEAD_TIME the candidate should still be returned (got '${out}')" +fi + +printf '\n %d passed, %d failed\n' "$PASS" "$FAIL" +[ "$FAIL" -eq 0 ] || exit 1 diff --git a/ci/tests/gate-decision.test.sh b/ci/tests/gate-decision.test.sh index e7096b2..f2fc2c9 100755 --- a/ci/tests/gate-decision.test.sh +++ b/ci/tests/gate-decision.test.sh @@ -42,7 +42,16 @@ cp "${REAL_CI_DIR}/gate.sh" "${REAL_CI_DIR}/lib.sh" "${REAL_CI_DIR}/versions.env # 15 of 47 assertions failed on `main` for that reason alone, including every one that expects a # PASS/UNDETERMINED/NOT-APPLICABLE headline. The suite was measuring the missing stub, not the # decision logic. `decisions-log` is added here in the same change that adds it to gate.sh. -for stage in spec-immutability decisions-log lint l0 render-golden l1 gitea-settings conformance; do +# +# AND IT HAPPENED AGAIN, with `approval-policy` (fixed 2026-10-01). Same shape, same cost: the +# stage was added to gate.sh by PR #17, never added here, so 15 of 50 assertions failed on `main` +# — measured on 0c7e8dd before touching anything, to be sure it was not the change under test. +# The note above was written about the previous instance and did not prevent the next one, which +# says the comment is not the fix. THE FIX IS TO DERIVE THE LIST: it now comes from gate.sh's own +# `run_stage` calls, so a stage added there can no longer be silently missing here. +mapfile -t STAGES < <(sed -n 's/^run_stage[[:space:]]\+\([a-z0-9-]\+\).*/\1/p' "${REAL_CI_DIR}/gate.sh") +[ "${#STAGES[@]}" -ge 8 ] || { echo "could not parse stage names out of gate.sh (got ${#STAGES[@]})" >&2; exit 1; } +for stage in "${STAGES[@]}"; do cat > "${CI}/${stage}.sh" <<'STUB' #!/usr/bin/env bash n="$(basename "$0" .sh)"; v="STAGE_RC_${n//-/_}" diff --git a/decisions/2026-10-01-admin-approval-override.md b/decisions/2026-10-01-admin-approval-override.md new file mode 100644 index 0000000..5b8dd09 --- /dev/null +++ b/decisions/2026-10-01-admin-approval-override.md @@ -0,0 +1,185 @@ +--- +date: 2026-10-01 +branch: fix/admin-approval-override +title: Administrators can merge their own green PR — required_approvals 1 → 0, plus an attributed /approve-override for sensitive paths +--- + +## What changed + +**The problem, measured first.** `required_approvals: 1` on `main` was a lock-out, not a +protection. Gitea refuses to let a pull request's author approve their own PR, and `jakman85` is +the **only** collaborator (permission `owner`). So no account in existence could approve a PR the +operator opened. Measured on the live API, 2026-10-01: + +| | | +|---|---| +| PR #35 (`bugfix/arr-to-plex`, author `jakman85`) | `gate / gate (pull_request)` = **success, 1m41s** | +| its reviews | `[]` | +| live `required_approvals` | `1` | +| live `block_admin_merge_override` | `true` | + +Green gate, zero reviews, no path to a merge — and the same was true of any PR that would have +fixed it. + +It was also never the design this repo wrote down. `ci/approval-policy.sh`'s own header and +`ci/gate.sh:194` both already described the model as *"`required_approvals` drops to 0 and the +GATE decides"*, and `infra/gitea/require-gate.sh`'s closing note named this exact failure and +prescribed this exact remedy. `decisions/2026-09-30-flux-deploy-notification-inhibited.md` had +flagged the 1-vs-0 disagreement as unresolved and not that session's to settle. This settles it +at 0. + +**Two layers were blocking, and both are addressed.** + +*Layer 1 — Gitea branch protection.* `required_approvals: 1 → 0` in +`infra/gitea/expected-branch-protection.json`, with the rationale in the file, plus +`infra/gitea/allow-admin-self-merge.sh` to apply it to the live rule (it reads the target **from +the declaration**, so the script cannot drift from what the gate checks). `require-gate.sh` was +changed to 0 as well — left at 1 it would silently re-impose the lock-out on its next run. + +The narrow choice matters: the *other* way to unblock an owner is +`block_admin_merge_override: false`, and that would have re-opened precisely what +`infra/gitea/block-admin-override.sh` was written to close (PRs #5–#9 merged with zero approvals +while the settings read as enforced). This change drops the approval **count** only. +`block_admin_merge_override` and `enable_status_check` both stay `true`, and the script asserts +both afterwards and fails loudly if the PATCH disturbed either. **A red gate remains unmergeable +by everyone, administrators included.** + +*Layer 2 — `ci/approval-policy.sh`.* Sensitive paths (`ci/**`, `infra/gitea/**`, `CLAUDE.md`, +kyverno, rbac, sops, Flux wiring) still require a non-author `APPROVED` review, which the sole +administrator still cannot obtain. An administrator may now waive it by commenting on the PR: + + /approve-override + +- the directive must **start a line** (a quoted or mid-sentence mention does not count, or + repeating someone else's override would re-arm it); +- a **reason is mandatory** — a bare directive records that review was waived but not why; +- the commenter's permission is **re-checked live** against + `/collaborators/{login}/permission` when the gate runs, so an old comment from a demoted + account carries nothing; +- it is **invalidated by a new commit** (the comment must post-date the head commit), mirroring + Gitea's own `dismiss_stale_approvals`; +- the overrider, timestamp and reason are printed in the gate log and the job summary, and a + self-waiver by the PR's own author is called out as such. + +Selection of the candidate comment is `ci/approval-override.py` — pure, offline, no authority of +its own — so it could be unit-tested; authorisation stays in the fail-closed stage. +`ci/tests/approval-override.test.sh` adds **23 assertions**, all passing, each constructing a +case the parser must refuse. Every new failure mode blocks: unreadable head-commit timestamp, +unreadable permission, parser error. + +**The cost, stated rather than buried.** For an administrator, *"a change to the gate cannot be +gated by the gate"* is now **advisory, not structural**. That is unavoidable with one human — the +alternative is a repository its owner cannot change — and what is bought back is an attributed, +reasoned, commit-scoped record of each waiver instead of a silent bypass. + +**The uncomfortable part, accepted with eyes open (operator decision).** Authorisation is by live +permission **alone**, with no allowlist of human accounts. The **agent account `gitea-admin` also +reports permission `owner`** on this repo (measured), so an agent *can* clear the sensitive-path +review on its own PR — and agents author most PRs here, so that is the common case, not an edge +case. I raised this before implementing; the operator chose permission-only. It is documented in +three places rather than left to be discovered: `approval-policy.json` +(`_known_consequence_agent_accounts`), the stage's header, and `CLAUDE.md`, which now tells +agents plainly **not** to use it on their own work. The narrowing path needs no code change — +add an `override_admins` list to `approval-policy.json` and the stage already requires membership +in addition to the permission check. + +Also updated: `CLAUDE.md` §1 and the §2 enforcement table (the "approval required" row is now +explicitly ⬜ with the reason, rather than a stale ✅), and `infra/gitea/README.md` gains §6 and +marks its historical step 4 as reversed — do not re-apply. + +**One adjacent fix, deliberately in scope: `ci/tests/gate-decision.test.sh` was already broken.** +It hand-maintains a list of stub stage scripts that "must name EVERY stage `gate.sh` runs", and it +did not name `approval-policy` — added to `gate.sh` by PR #17 and never added here. `run_stage` +therefore got exit 127 from a missing file and recorded a FAILED stage in every case, so **15 of +50 assertions failed**, including every one expecting a PASS/UNDETERMINED headline. Measured on +pristine `origin/main` (`0c7e8dd`) before touching it, so the attribution is not a guess. + +I fixed it because `make -C ci test` is the *only* runner for the suite this change adds — leaving +it red would have made the new tests unverifiable in practice. The file's own comment already +narrated this exact failure for `gitea-settings` and did not prevent the repeat, which says the +comment was not the fix. So the stub list is now **derived from `gate.sh`'s own `run_stage` lines** +and asserts it parsed at least 8 of them: a stage added to the gate can no longer be silently +missing from its test. `make -C ci test` goes from **FAILED to all suites passed**. + +## What I verified + +- **The lock-out is real, and PR #35 is the proof.** Queried live: head sha + `8a094c46`, statuses `gate / gate (pull_request) = success :: Successful in 1m41s`, + `/pulls/35/reviews` → `[]`, `/branch_protections/main` → `required_approvals: 1`. I read the + **rule object**, not `/branches/main`, per §2b. +- **`jakman85` is the only collaborator**: `/collaborators` → one entry. Permission `owner`. +- **`gitea-admin` also reports `owner`** — this is why the agent-account consequence above is a + statement of fact and not a hypothetical. +- **PR #35 touches no sensitive paths** (`apps/wave-0/plex.yaml`, `apps/wave-1/{radarr,sonarr}.yaml`, + `catalog/{radarr,sonarr}.yaml`, its decisions entry) — so `ci/approval-policy.sh` was never its + blocker. Layer 1 alone unblocks it. +- **API shapes verified before depending on them**, rather than assumed: `/git/commits/{sha}` + returns `.commit.committer.date` as RFC3339 **with an offset** (`2026-10-01T23:09:08-05:00`), + not a trailing `Z` — a parser handling only `Z` would have silently dropped every real + override, so there is a test for the offset form. PR conversation comments are under + `/issues/{n}/comments`; `/pulls/{n}/comments` returns diff comments only and would never have + seen an override. +- **`ci/tests/approval-override.test.sh`: 23 passed, 0 failed.** Covers the happy path, bare + directive, mid-sentence and `>`-quoted directives, stale and equal-timestamp overrides, + unparseable timestamps, missing login, invalid JSON, non-list JSON, latest-wins, offset + timestamps, and a configurable directive. +- `bash -n` clean on `ci/approval-policy.sh`, `infra/gitea/allow-admin-self-merge.sh`, + `infra/gitea/require-gate.sh`; `jq -e .` clean on both JSON declarations. +- **`gate-decision.test.sh`'s 15 failures are pre-existing, measured not assumed.** Extracted + `origin/main`'s `ci/` at `0c7e8dd` into a throwaway directory and ran the suite from there: + **FAIL, 15 failed / 35 passed**. After deriving the stub list from `gate.sh`: **PASS, 50 + assertions**. +- **`make -C ci test` in the pinned toolbox: `ci/tests: all suites passed`, rc 0** — approval-override + 23, gate-decision 50, touched-requirements-base 31, test-attribution 15, touched-requirements 26, + decisions-log pass. It was rc 2 (`ci/tests: FAILED`) before the stub-list fix. +- **Toolbox merge gate** (`./infra/devcontainer/dev.sh run bash -c './ci/gate.sh'`), run on the + rebased branch: **rc 3, `MERGE GATE: UNDETERMINED`**. Green stages: spec-immutability, + decisions-log, lint, l0 (23 offline cases, 0 failures), render-golden including the new + `catalog/render.py --selftest`, l1 (3 cases), approval-policy (N/A offline). Skipped: + `gitea-settings` (no `SETTINGS_API_TOKEN` on this host) and `conformance` (no cluster). + **This is NOT a green run and I am not reporting it as one** — rc 3 is UNDETERMINED per the + CLAUDE.md §3 contract. + +## What I did NOT verify + +- **The live Gitea rule still says `required_approvals: 1`. I did not change it.** Mutating live + platform settings is an operator action; `infra/gitea/allow-admin-self-merge.sh` is written, + syntax-checked, and **unrun**. The declaration and the live rule must move together. + *Now measured, not predicted:* PR #38's CI run (run 202, job 0) fails at **1m44s** with + `required_approvals: DECLARED 0 but LIVE is 1` → `gitea-settings: FAIL`. Correct behaviour, and + the PR stays red until the script is run. +- **The override has never been ACCEPTED by a real run** — only offered. Measured in the same CI + log: the stage reached `approval-policy: FAIL` on the sensitive paths and printed the *new* + code's guidance, `Or, if you hold admin/owner on this repository, comment on the PR: + /approve-override ... NEWER than head commit 7b9f733c`. So the new stage **is** what + runs on this PR (self-bootstrapping confirmed by measurement, not by reasoning), and the + authorising branch — comment parse → live permission lookup → exit 0 — is still **unexercised**. + Posting the comment on PR #38 is its first real test. If it does not work, the recovery is to + PATCH `required_approvals` and read run 202's successor log. +- **I have not seen the gate still REFUSE a merge** now that approvals are 0. The claim that + `block_admin_merge_override: true` keeps a red gate unmergeable rests on Gitea's documented + behaviour and on the 2026-09-29 session, **not** on a provoked refusal in this one. That probe + is written out at the foot of `allow-admin-self-merge.sh` and should be done, exactly as the + push rejection was provoked rather than assumed. +- **Clock skew in the staleness check is unhandled.** Staleness compares the comment's + server-side `created_at` against the head commit's *committer* date, which comes from the + committer's machine. A commit dated in the future would reject legitimate overrides until real + time catches up (fail-closed, so loud, but annoying). Gitea's own `dismiss_stale_approvals` + compares SHAs instead; a comment cannot. Not hit here — PR #35's commit date is ~21 min behind + server time — and not fixed. +- **`ci/tests/approval-override.test.sh` does not run in the merge gate.** `ci/gate.sh` runs no + `ci/tests/*`; they run only via `make -C ci test`. So a regression in the parser would not be + caught by CI. PR #33 (merged while this branch was being written, and rebased onto here) fixed + the *same class* of gap for `catalog/render.py --selftest` by invoking it from + `ci/render-golden.sh` — it did **not** wire `ci/tests/*` in, which I confirmed after rebasing + rather than assuming. Wiring them is a one-line stage and I did not widen scope to it. +- **Authorisation itself is untested code.** `ci/approval-override.py` is unit-tested; the live + permission lookup, the `override_admins` allowlist branch, and every new BLOCK path in + `ci/approval-policy.sh` need Gitea and have never been exercised. The allowlist branch in + particular is implemented but **unused** — `override_admins` is absent by design. +- **The other open PRs will go red** once the live value changes, until they are rebased onto the + merged declaration: as of writing those are **#29** and **#37** (both `gitea-admin`), plus + **#35**. I did not rebase them. Merging them first avoids it entirely; the script's header gives + the order. PRs #33, #34 and #36 merged while this branch was being written — this branch is + rebased onto `0c7e8dd`, so the list was re-measured rather than carried forward. +- No cluster state was read or changed; nothing here touches Flux, Kyverno or any workload. diff --git a/infra/gitea/README.md b/infra/gitea/README.md index caf48ad..464876b 100644 --- a/infra/gitea/README.md +++ b/infra/gitea/README.md @@ -52,7 +52,7 @@ All four steps are complete and each was verified, not assumed: | 1. Runner registered | ✅ `incluster-dind` online. Root cause of its absence was a stale `/data/.runner` identity; clearing the PVC forced re-registration | | 2. Gate observed green on a real PR | ✅ `gate / gate (pull_request)` ~1m35s | | 3. `enable_status_check: true` | ✅ context `gate / gate (pull_request)` — that one ONLY | -| 4. Collaborator + approvals | ✅ `jakman85` is owner; `required_approvals: 1` | +| 4. Collaborator + approvals | ✅ `jakman85` is owner. `required_approvals` was raised to 1 here and **lowered back to 0 on 2026-10-01** — see §6, it was a lock-out | Direct push to `main` was **provoked and rejected** (`pre-receive hook declined`) as `gitea-admin`, a site admin — so push protection is not merely declared, it holds for the @@ -124,6 +124,55 @@ the stage goes **green on a real PR**, and only then set `REQUIRE_GITEA_SETTINGS **Changing the declaration is how you change policy.** If the live state is what you now want, edit this JSON in the same commit — deliberately, in review — rather than letting the two drift. +### 6. `allow-admin-self-merge.sh` — undoing the lock-out step 4 created (2026-10-01) + +Step 4 above raised `required_approvals` to 1. That was a **lock-out, not a protection**, and the +note at the bottom of `require-gate.sh` had already said why: *Gitea will not let an author approve +their own PR.* With `jakman85` the only collaborator, there was no account in existence that could +approve a PR `jakman85` opened. + +Measured 2026-10-01 — **PR #35**: `gate / gate (pull_request)` **success in 1m41s**, reviews `[]`, +`required_approvals: 1`, `block_admin_merge_override: true`. Unmergeable, permanently, with no +action available to its author. The same was true of any PR fixing it. + +It was also never the documented design. `ci/approval-policy.sh`'s header and `ci/gate.sh:194` +both already describe the model as *"`required_approvals` drops to 0 and the GATE decides"*, and +`decisions/2026-09-30-flux-deploy-notification-inhibited.md` flagged the 1-vs-0 disagreement as +unresolved and not that session's to settle. So this is a correction toward the stated intent, not +a weakening away from it. + + ./infra/gitea/allow-admin-self-merge.sh # required_approvals -> 0 (read from the declaration) + +**What stays switched on, and why that is the point.** The other way to unblock an owner is +`block_admin_merge_override: false`, and it would have been the wrong one — that re-opens exactly +what §5 above closed, letting the one account that merges bypass the gate itself. This script +touches the approval **count** only and *verifies* afterwards that `block_admin_merge_override` and +`enable_status_check` are both still `true`, failing loudly if the PATCH disturbed either: + +| | before | after | +|---|---|---| +| `required_approvals` | 1 | **0** | +| `block_admin_merge_override` | true | true (asserted) | +| `enable_status_check` | true | true (asserted) | + +So a **red gate is still unmergeable by everyone, administrators included**. What changed is that +a *green* one no longer needs a signature that cannot exist. + +**Security-relevant changes are still reviewed** — by `ci/approval-policy.sh` against +`approval-policy.json` `sensitive_paths`, not by this field. An administrator waives that review +only by posting `/approve-override ` on the PR: reason mandatory, permission re-checked +live, invalidated by a new commit, and recorded by account in the gate summary. Note that the +**agent account `gitea-admin` also reports permission `owner`**, so it can clear its own PRs that +way; that is an accepted consequence of authorising by permission alone, and the route to +narrowing it (an `override_admins` allowlist, already honoured by the code) is written down in +`approval-policy.json`. + +**Sequencing.** `ci/gitea-settings.sh` compares live against the declaration *from the branch under +test*, and `REQUIRE_GITEA_SETTINGS=true`. So merge any other open PRs **before** running this, then +run it, then re-run the gate on the branch carrying the new declaration. Any PR left open across +the change declares 1 against a live 0 and goes **red until rebased** — the detector working, not +a fault. The script's header spells the order out. + ## `conformance-trigger-kubeconfig.sh` — the narrow L2/L3 trigger credential Implements `docs/plans/l2-l3-conformance-ci-wiring.md` §3 item 6 (= §5 step 7): mint a @@ -204,6 +253,8 @@ it. The name matters — see the note under the behaviour table above.) 2. **Watch the gate actually report on a real PR**, and note the context name it produced. 3. **Only then set `enable_status_check: true`** with that exact context. 4. Add the human reviewer as a repo collaborator, then raise `required_approvals` to 1. + — **Step 4 was REVERSED on 2026-10-01 (§6).** It assumed a reviewer distinct from the author + would exist. One never did, so this step produced a lock-out. Do not re-apply it. > **Do not do step 3 before step 1.** A required status check with no runner leaves every > PR permanently pending — including the PR that would fix it. That half is measured: PR #1 diff --git a/infra/gitea/allow-admin-self-merge.sh b/infra/gitea/allow-admin-self-merge.sh new file mode 100755 index 0000000..5c5e0dd --- /dev/null +++ b/infra/gitea/allow-admin-self-merge.sh @@ -0,0 +1,105 @@ +#!/usr/bin/env bash +# Let the sole administrator merge their own green PR: required_approvals 1 -> 0 on `main`. +# +# WHY. `required_approvals: 1` plus Gitea's refusal to let an author approve their own pull +# request plus exactly ONE collaborator (`jakman85`, permission `owner`) is a lock-out, not a +# policy. Every PR the operator opened was permanently unmergeable, including the PR that would +# fix it. Measured 2026-10-01: PR #35 — `gate / gate (pull_request)` success in 1m41s, zero +# reviews, `required_approvals: 1`, no account in existence able to satisfy it. +# +# The approvals field was also never what the design said it was. `ci/approval-policy.sh`'s +# header and `ci/gate.sh:194` both already described the model as "required_approvals: 0 and the +# GATE decides", and `require-gate.sh`'s own closing note named this exact failure and this exact +# remedy. decisions/2026-09-30-flux-deploy-notification-inhibited.md flagged the 1-vs-0 +# disagreement as unresolved. This resolves it at 0. +# +# WHAT THIS DOES *NOT* DO, and the distinction is the whole point. +# +# * It does NOT touch `block_admin_merge_override`, which stays TRUE. The required status check +# still binds ABSOLUTELY — a red gate is unmergeable by everyone, administrators included. +# Setting `block_admin_merge_override: false` would have been the other way to unblock an +# owner, and it would have re-opened precisely what block-admin-override.sh was written to +# close: PRs #5-#9 merged with zero approvals while the settings read as enforced. +# * It does NOT stop reviewing security-relevant changes. Those are gated by +# ci/approval-policy.sh against infra/gitea/approval-policy.json `sensitive_paths`, which an +# administrator can waive only with an explicit, attributed `/approve-override ` +# comment on the PR. +# +# ORDER MATTERS — READ THIS BEFORE RUNNING IT. +# +# ci/gitea-settings.sh fails the merge gate on ANY divergence between the live rule and +# infra/gitea/expected-branch-protection.json, and `gate.yml` sets REQUIRE_GITEA_SETTINGS=true. +# The declaration in git now says 0. So between running this and merging that declaration, the +# live value and the declared value must be compared from a branch that CARRIES the new +# declaration — otherwise the stage goes red, correctly, for drift. +# +# 1. MERGE any other open PRs FIRST, while approvals still work for them (their branches +# declare 1, and a PR you did not author can still be approved normally). Skipping this +# only costs a rebase later; it does not break anything. +# 2. Run this script. +# 3. Re-run the gate on the branch that carries the new declaration. `gitea-settings` goes +# green because live 0 == declared 0. +# 4. Merge that branch. +# 5. Any PR still open from before step 2 now declares 1 against a live 0 and will be RED +# until it is rebased onto the new main. That is the detector doing its job, not a fault. +# +# Recoverable: `required_approvals` can be PATCHed back to 1 by an owner at any time. If you do, +# change expected-branch-protection.json in the same breath or every PR goes red on drift. +set -euo pipefail +HOST=gitea.home.keithjackson.dev; OWNER=Home-Network; REPO=home-network-k3s-impl +DECL="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/expected-branch-protection.json" + +command -v jq >/dev/null || { echo "jq is required" >&2; exit 1; } +[ -f "$DECL" ] || { echo "declaration missing: ${DECL}" >&2; exit 1; } + +# Take the target from the DECLARATION, not from a literal here. If the two could disagree, +# this script would be one more thing re-asserting a value nothing checks — which is the +# failure mode the whole of infra/gitea/ exists to stop. +WANT="$(jq -r '.protection.required_approvals' "$DECL")" +[[ "$WANT" =~ ^[0-9]+$ ]] || { echo "declaration has no numeric required_approvals" >&2; exit 1; } + +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")" +[ -n "$U" ] && [ -n "$P" ] || { echo "no stored credential for $HOST" >&2; exit 1; } +echo "authenticating as '${U}' (secret not shown)" +api() { curl -sS --config <(printf 'user = "%s:%s"\n' "$U" "$P") "$@"; } +B="https://${HOST}/api/v1/repos/${OWNER}/${REPO}/branch_protections/main" + +# Read the RULE OBJECT, never /branches/main: the branch view does not expose +# block_admin_merge_override and showed all-green for hours while the rule was overridable. +echo "==> before" +api "$B" | jq -r '{required_approvals, block_admin_merge_override, enable_status_check, status_check_contexts}' + +echo "==> setting required_approvals = ${WANT} (declared in expected-branch-protection.json)" +api -X PATCH -H 'Content-Type: application/json' \ + -d "{\"required_approvals\": ${WANT}}" "$B" >/dev/null + +echo "==> after" +AFTER_JSON="$(api "$B")" +jq -r '{required_approvals, block_admin_merge_override, enable_status_check, status_check_contexts}' <<<"$AFTER_JSON" + +GOT="$(jq -r '.required_approvals' <<<"$AFTER_JSON")" +ADMIN_BLOCK="$(jq -r '.block_admin_merge_override' <<<"$AFTER_JSON")" +STATUS="$(jq -r '.enable_status_check' <<<"$AFTER_JSON")" + +fail=0 +[ "$GOT" = "$WANT" ] || { echo "PATCH did not take: required_approvals is ${GOT}, wanted ${WANT}" >&2; fail=1; } +# These two are what keeps this change narrow. If the PATCH disturbed either, the gate has +# stopped binding and that must be loud, not discovered later. +[ "$ADMIN_BLOCK" = true ] || { echo "block_admin_merge_override is ${ADMIN_BLOCK}, MUST be true — the gate is now bypassable" >&2; fail=1; } +[ "$STATUS" = true ] || { echo "enable_status_check is ${STATUS}, MUST be true — the gate is no longer required" >&2; fail=1; } +[ "$fail" -eq 0 ] || { echo "do NOT record this as applied." >&2; exit 1; } + +cat <<'NOTE' + +NOW PROVE IT -- the setting is not the evidence. Two things to see, not assume: + + 1. An APPROVAL IS NO LONGER NEEDED. On your own PR with a green gate, the merge button is + live with zero reviews on it. + 2. THE GATE STILL BINDS. On a PR whose gate is RED or still pending, Gitea must REFUSE the + merge -- for you, as owner. If it lets you through, block_admin_merge_override is not + doing its job and this change has traded a lock-out for a bypass. + +Then confirm the detector agrees: run ci/gate.sh from a branch carrying the new +expected-branch-protection.json and check that `gitea-settings` is green rather than DRIFT. +NOTE diff --git a/infra/gitea/approval-policy.json b/infra/gitea/approval-policy.json index 4890c62..14e7fc1 100644 --- a/infra/gitea/approval-policy.json +++ b/infra/gitea/approval-policy.json @@ -1,6 +1,6 @@ { "_intent": "The CI/CD pipeline is the approver. A human approval is required ONLY for changes that alter what the pipeline itself verifies, or the security posture it rests on -- because those are the changes CI cannot honestly gate: a PR that weakens the gate would be approved by the weakened gate.", - "_rule": "If a PR touches any sensitive_paths glob, it needs an APPROVED review from someone who is not its author. Everything else merges on a green gate alone.", + "_rule": "If a PR touches any sensitive_paths glob, it needs an APPROVED review from someone who is not its author, OR an admin override (see .override). Everything else merges on a green gate alone.", "_on_failure_to_evaluate": "BLOCK. Unlike gitea-settings, a skip here is a bypass, not a gap.", "sensitive_paths": [ "ci/**", @@ -24,5 +24,17 @@ "platform/kyverno/**": "the admission policy engine", ".sops.yaml, **/*.sops.yaml, **/rbac.yaml": "encryption recipients, secrets, and cluster authority", "CLAUDE.md": "the delivery contract agents follow" + }, + "override": { + "_why": "This repository has ONE human. Gitea refuses to let an author approve their own pull request, so 'a non-author APPROVED review' is a requirement the sole administrator can never satisfy -- every sensitive-path PR they open was permanently unmergeable, including the PR that would fix it. Measured 2026-10-01: PR #35 green gate, zero reviews, unmergeable. An escape hatch that the administrator can actually reach is therefore REQUIRED for the repository to function; the design question is only whether it leaves a record. This one does.", + "enabled": true, + "comment_directive": "/approve-override", + "requires_reason": true, + "authorized_permissions": ["admin", "owner"], + "invalidated_by_new_commits": true, + "_how": "Comment on the PR with the directive at the START of a line, followed by a reason: '/approve-override '. ci/approval-policy.sh resolves the commenter's LIVE repo permission via /collaborators/{login}/permission and accepts the override only if it is in authorized_permissions. The overrider, the timestamp and the reason are printed in the gate log and the job summary.", + "_self_override_is_allowed": "The author may override their own PR if they hold an authorized permission. That is the entire point -- with one human there is no one else -- but it means the 'a change to the gate cannot be gated by the gate' protection is, for an administrator, advisory rather than structural. It is traded for an auditable record: the waiver is an explicit act, attributed, with a stated reason, in the PR's comment history and the gate output.", + "_known_consequence_agent_accounts": "ACCEPTED, EYES OPEN (operator decision, 2026-10-01). Authorisation is by live permission ALONE, with no allowlist of human accounts. The agent account `gitea-admin` reports permission `owner` on this repo (measured 2026-10-01), so an AGENT can clear the sensitive-path review on its own PR by posting the directive. Agents author most PRs here, so this is the broad case, not the edge case. To narrow it later without changing any logic, add an `override_admins` allowlist here and require membership in addition to the permission check -- ci/approval-policy.sh reads both from this file.", + "_staleness": "An override is dropped unless its comment is NEWER than the PR head commit, mirroring Gitea's own dismiss_stale_approvals. An override earned at commit A therefore does not silently cover commit B; push again and you must override again." } } diff --git a/infra/gitea/expected-branch-protection.json b/infra/gitea/expected-branch-protection.json index da88569..010f292 100644 --- a/infra/gitea/expected-branch-protection.json +++ b/infra/gitea/expected-branch-protection.json @@ -1,6 +1,9 @@ { "_comment": "DECLARED desired state of Gitea branch protection on main. ci/gitea-settings.sh fails the merge gate when the LIVE Gitea diverges from this. Gitea settings live in Gitea database - invisible to Flux, unreadable by every other check - so this file plus that check are the only drift detection that exists for them.", "_generated": "2026-09-29 from the live rule, after the session that found protection OFF and zero runners for a month", + "_amended": "2026-10-01: required_approvals 1 -> 0. See _why_required_approvals_is_zero.", + "_why_required_approvals_is_zero": "The GATE is the approver (operator intent, 2026-09-29). required_approvals: 1 was never consistent with that intent: ci/approval-policy.sh's header and ci/gate.sh:194 both already described the model as 'required_approvals: 0 and the GATE decides', and infra/gitea/require-gate.sh's own closing note named the consequence of 1 -- 'Gitea will not let an author approve their own PR'. With exactly ONE collaborator (jakman85, owner), that made every PR the operator opened permanently unmergeable. Measured 2026-10-01: PR #35, gate green in 1m41s, zero reviews, no path to a merge. decisions/2026-09-30-flux-deploy-notification-inhibited.md had already flagged the 1-vs-0 disagreement as unresolved; this settles it at 0.", + "_what_still_holds_at_zero": "This drops the APPROVAL COUNT only. block_admin_merge_override stays TRUE and enable_status_check stays TRUE, so the required gate context still binds absolutely -- nobody, administrator included, can merge a red gate. That distinction is the whole reason this is the chosen fix rather than block_admin_merge_override: false, which would have handed admins a bypass over the gate itself and re-opened what infra/gitea/block-admin-override.sh was written to close (PRs #5-#9, merged with zero approvals). Architecturally and security-relevant paths are still reviewed, by ci/approval-policy.sh rather than by this field.", "branch": "main", "runners_online_min": 1, "protection": { @@ -11,7 +14,7 @@ "status_check_contexts": [ "gate / gate (pull_request)" ], - "required_approvals": 1, + "required_approvals": 0, "enable_approvals_whitelist": false, "block_admin_merge_override": true, "block_on_rejected_reviews": true, diff --git a/infra/gitea/require-gate.sh b/infra/gitea/require-gate.sh index 4eb21f5..54e13ef 100755 --- a/infra/gitea/require-gate.sh +++ b/infra/gitea/require-gate.sh @@ -34,7 +34,7 @@ jq -n '{ enable_push: false, enable_status_check: true, status_check_contexts: ["gate / gate (pull_request)"], - required_approvals: 1, + required_approvals: 0, block_on_rejected_reviews: true, dismiss_stale_approvals: true, block_on_outdated_branch: false, @@ -48,11 +48,21 @@ api "https://${HOST}/api/v1/repos/${OWNER}/${REPO}/branches/main" \ cat <<'NOTE' -NOTE ON required_approvals: 1 -- Gitea will not let an author approve their own PR. - * Agent-opened PRs (author gitea-admin) are approved by jakman85. Fine. - * PRs YOU open as jakman85 need gitea-admin to approve. If that is awkward, either open - agent work as gitea-admin and review as jakman85, or set required_approvals back to 0 - and rely on the status check alone. +NOTE ON required_approvals: 0 -- CHANGED FROM 1 ON 2026-10-01, and this script is the reason +the change had to land here too: re-running it with 1 would silently re-impose the lock-out. + +The note that used to sit here called the problem and its own fix: "Gitea will not let an +author approve their own PR ... if that is awkward ... set required_approvals back to 0 and +rely on the status check alone." It was awkward. With jakman85 the ONLY collaborator, every PR +the operator opened was unmergeable -- measured on PR #35, gate green in 1m41s, zero reviews, +nothing that could ever satisfy the rule. So: 0, and the status check alone, as written. + +What did NOT change, deliberately: + * block_admin_merge_override stays TRUE. This drops the approval COUNT; it does not hand + anyone a bypass over the gate. A red gate is still unmergeable by everybody. + * Architecturally/security-relevant paths are still human-reviewed -- by ci/approval-policy.sh + (sensitive_paths in infra/gitea/approval-policy.json), which an administrator can waive + only by posting an explicit, attributed `/approve-override ` comment. Then prove it, rather than trusting the API: git commit --allow-empty -m probe && git push origin main # EXPECT: rejected