The override did not work on its first real test, and the reason was invisible from the outside. Run 203 on PR #38: a well-formed `/approve-override` posted by jakman85 in the PR comment box, 29 minutes newer than the head commit, and the stage still blocked with: approval-override: COMMENTS_JSON is not a list SETTINGS_API_TOKEN is minted [read:repository, read:admin], and Gitea puts /issues/{n}/comments behind the *issue* scope — so Gitea answered with an error OBJECT, `jq -e .` accepted it as valid JSON, and the stage fell through to 'no approving review'. A permission failure wearing the costume of a missing override. /pulls/{n}/files and /pulls/{n}/reviews are under read:repository, which is why every other part of the same stage worked. 1. The diagnosis no longer lies. COMMENTS_JSON leaves the must-parse-or-block loop and is checked for being an ARRAY; when it is not, the stage prints Gitea's own .message, names the missing scope and says where to post instead. An unreadable source can only cause a false BLOCK, never a false pass, so a loud warning is the honest behaviour and blocking was not. 2. An override is accepted from a PR REVIEW body as well as a comment, so the hatch is not hostage to one API scope — a review of type 'Comment' works on the token CI has today. The gate names which source it used; PENDING (unsubmitted, author-only) reviews are dropped. 3. set-ci-token.sh requests read:issue, making the ordinary comment box work — on a RE-RUN only, since an existing token's scopes cannot be widened. ci/tests/approval-override.test.sh: 23 -> 38 assertions, covering the reviews source, PENDING reviews, an error object from the comments API, 'an unreadable comments source must not suppress a valid review override', latest-wins across both sources, and that the diagnostic quotes Gitea's message. Also measured and recorded: run 203 shows gitea-settings PASS after the operator ran the live PATCH, with block_admin_merge_override still true — the narrow change held, the approval count moved and the gate's teeth did not. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
195 lines
8.1 KiB
Python
Executable File
195 lines
8.1 KiB
Python
Executable File
#!/usr/bin/env python3
|
|
"""ci/approval-override.py — select the admin-override on a PR, if there is a valid one.
|
|
|
|
This script decides NOTHING about authority. It parses a PR's comments and reviews 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 OR a review 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
|
|
|
|
TWO SOURCES, AND WHY THAT IS NOT BELT-AND-BRACES. Measured 2026-10-02 on PR #38: the override was
|
|
posted as an ordinary PR comment, was well-formed, was newer than the head commit — and the stage
|
|
still blocked, because CI could not READ it. `SETTINGS_API_TOKEN` is minted with scopes
|
|
`[read:repository, read:admin]`, and Gitea puts /issues/{n}/comments behind the *issue* scope, so
|
|
the request came back as an error OBJECT rather than an array. `/pulls/{n}/reviews` is under
|
|
read:repository and was readable in the same run. So:
|
|
|
|
* a REVIEW body works with the token CI has today (leave a review of type "Comment");
|
|
* a COMMENT body works once the token carries read:issue (see infra/gitea/set-ci-token.sh).
|
|
|
|
Accepting both means the hatch is not hostage to one API scope. A source that cannot be read
|
|
contributes no candidates and is reported on stderr; it can only cause a false BLOCK, never a
|
|
false pass, which is why degrading it is safe and blocking on it was not necessary.
|
|
|
|
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. Lines beginning with '>' are DROPPED
|
|
rather than unquoted, for exactly that reason.
|
|
* 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 approvals. Push new commits and
|
|
you must re-override.
|
|
* A PENDING REVIEW IS NOT SUBMITTED. It is a private draft visible only to its author, so it is
|
|
not an act of anything and is dropped.
|
|
* THE LATEST CANDIDATE WINS, across both sources, so a later override supersedes an earlier one.
|
|
|
|
Output: one TSV line (login, timestamp, source, 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 (or an error object; or absent)
|
|
REVIEWS_JSON the Gitea /pulls/{n}/reviews array (or an error object; or absent)
|
|
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 load_source(env_name, label):
|
|
"""Return a list of items for one source, or [] with a stderr note if it is unusable."""
|
|
raw = os.environ.get(env_name)
|
|
if not raw or not raw.strip():
|
|
return []
|
|
try:
|
|
parsed = json.loads(raw)
|
|
except (ValueError, TypeError):
|
|
print(
|
|
"approval-override: %s is not valid JSON — ignoring that source" % label,
|
|
file=sys.stderr,
|
|
)
|
|
return []
|
|
if isinstance(parsed, list):
|
|
return parsed
|
|
# An error object, typically {"message": "token does not have ... scope"}. Say which,
|
|
# because "no override found" and "could not read the overrides" look identical from
|
|
# the outside and the first wasted a real debugging session.
|
|
detail = ""
|
|
if isinstance(parsed, dict):
|
|
detail = str(parsed.get("message") or parsed.get("errors") or "")[:200]
|
|
print(
|
|
"approval-override: %s is not a list (Gitea returned %s%s) — ignoring that source"
|
|
% (label, type(parsed).__name__, ": " + detail if detail else ""),
|
|
file=sys.stderr,
|
|
)
|
|
return []
|
|
|
|
|
|
def candidates_from(items, source, directive, head_time):
|
|
"""Yield (when, login, reason, raw_when, source) for every valid override in one source."""
|
|
found = []
|
|
for item in items:
|
|
if not isinstance(item, dict):
|
|
continue
|
|
login = ((item.get("user") or {}).get("login") or "").strip()
|
|
reason = extract_reason(item.get("body"), directive)
|
|
if not login or not reason:
|
|
continue
|
|
# A PENDING review is an unsubmitted private draft — not an act of anything.
|
|
state = str(item.get("state") or "").upper()
|
|
if source == "review" and state == "PENDING":
|
|
print(
|
|
"approval-override: dropping PENDING (unsubmitted) review by '%s'" % login,
|
|
file=sys.stderr,
|
|
)
|
|
continue
|
|
raw_when = (
|
|
item.get("submitted_at") or item.get("created_at") or item.get("updated_at") or ""
|
|
)
|
|
when = parse_ts(raw_when)
|
|
if when is None:
|
|
print(
|
|
"approval-override: dropping %s override by '%s' — unreadable timestamp %r"
|
|
% (source, login, raw_when),
|
|
file=sys.stderr,
|
|
)
|
|
continue
|
|
# An override must post-date the commit it covers.
|
|
if head_time is not None and when <= head_time:
|
|
print(
|
|
"approval-override: dropping STALE %s override by '%s' (%s) — not newer than "
|
|
"head commit (%s). Re-post it to override the current commit."
|
|
% (source, login, when.isoformat(), head_time.isoformat()),
|
|
file=sys.stderr,
|
|
)
|
|
continue
|
|
found.append((when, login, reason, raw_when, source))
|
|
return found
|
|
|
|
|
|
def main():
|
|
directive = os.environ.get("DIRECTIVE", "/approve-override").strip()
|
|
head_time = parse_ts(os.environ.get("HEAD_TIME", ""))
|
|
|
|
found = []
|
|
found += candidates_from(
|
|
load_source("COMMENTS_JSON", "COMMENTS_JSON"), "comment", directive, head_time
|
|
)
|
|
found += candidates_from(
|
|
load_source("REVIEWS_JSON", "REVIEWS_JSON"), "review", directive, head_time
|
|
)
|
|
|
|
if not found:
|
|
return 0
|
|
|
|
# Latest wins, across both sources: a later override supersedes an earlier one.
|
|
when, login, reason, raw_when, source = max(found, 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\t%s" % (login, raw_when, source, reason))
|
|
return 0
|
|
|
|
|
|
if __name__ == "__main__":
|
|
sys.exit(main())
|