timothyandtimothy
761e575836
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 6s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 21s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m45s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m22s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 5m56s
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m33s
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
1116 lines
62 KiB
Python
1116 lines
62 KiB
Python
"""The scheduled-auto-merge path verifies the protection it rests on (ersatztv#778).
|
|
|
|
`merge_when_checks_succeed` hands the actual merge to Gitea, to be performed later against whatever
|
|
head is green at that moment. Everything the hook proves is therefore a SNAPSHOT. What makes that
|
|
safe is stated in the hook and in #622: `review-verdict/h10` is a REQUIRED status check on the base
|
|
branch, a commit status belongs to exactly one sha, so a commit pushed after scheduling cannot
|
|
inherit the verdict and Gitea's own gate refuses the merge.
|
|
|
|
That guarantee is branch-protection CONFIG. It lives outside this repo, and before #778 nothing
|
|
compared the two — the hook asserted it in a comment and in the reason string a human reads, which
|
|
is a claim about the past, not a check. These tests pin the conversion of that assumption into a
|
|
precondition.
|
|
|
|
The outcome set is the contract, and each arm is asserted separately because collapsing any two of
|
|
them is how this class of guard has failed here before:
|
|
|
|
* required check PRESENT -> proceed (no opinion drawn from this check)
|
|
* branch protection UNREADABLE -> ask (a transient failure is not evidence of safety)
|
|
* required check ABSENT -> deny (this is #622's hole reopened, not a degraded read)
|
|
* a GLOB rule COULD govern the base -> ask, and distinctly from the unreadable case. This hook
|
|
does not reimplement Gitea's glob dialect, so "some rule might apply and we cannot tell" is a
|
|
fourth answer, not a flavour of the third. Tests that assert only `"ask" in reason` cannot tell
|
|
the two apart — and a crashed classifier also produces an ask — so each arm is pinned on the
|
|
text unique to it.
|
|
|
|
Observable contract: the hook exits 0 with EMPTY stdout when it has no opinion, and emits a JSON
|
|
`permissionDecision` otherwise.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import json
|
|
import os
|
|
import shutil
|
|
import subprocess
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
REPO_ROOT = Path(__file__).resolve().parents[2]
|
|
HOOK = REPO_ROOT / ".claude" / "hooks" / "pretooluse-merge-consent.sh"
|
|
|
|
SHA = "a9e3e23abf337980ca4c05854f5b1e210099d08b"
|
|
SHORT = SHA[:7]
|
|
|
|
# The PR is deliberately NOT docs-only: the docs-only exemption short-circuits the whole gate before
|
|
# the scheduled-merge branch is reached, so a docs PR would pass these tests without ever running
|
|
# the code under test.
|
|
CURL_SHIM = r"""#!/usr/bin/env python3
|
|
import json, os, sys, pathlib, urllib.parse
|
|
|
|
state = pathlib.Path(os.environ["STUB_DIR"])
|
|
# The repo's OWN committed snapshot of `main`'s required checks (ersatztv#787). The arms that model
|
|
# a healthy, grantable repo serve exactly this list, so the hook's guard-scope freshness check sees
|
|
# `match` and the grant path stays reachable. Read from the file rather than restated here: a second
|
|
# copy would drift the day the real required checks change, and these tests would then be asserting
|
|
# that a grant happens in a repo state that no longer exists.
|
|
REQUIRED = json.loads(pathlib.Path(os.environ["REQUIRED_CONTEXTS_JSON"]).read_text())["contexts"]
|
|
args = sys.argv[1:]
|
|
url = [a for a in args if a.startswith("http")][-1]
|
|
|
|
# The branch-protection call uses `-o <file> -w '%{http_code}'` rather than `curl -sf`, precisely so
|
|
# it can tell a 200 from every other outcome (`curl -sf` collapses every HTTP error into exit 22
|
|
# with empty output, hiding the difference between an empty list and a failed request). It does
|
|
# NOT treat 404 as a finding. The shim must therefore behave like real curl for those
|
|
# flags: body to the -o file, status code to stdout. A shim that ignored them would make the hook
|
|
# read an empty body and a blank code on EVERY path, and the tests would pass by accident against a
|
|
# guard that never ran — the "test double's fidelity claim" failure this repo has on record.
|
|
def respond(body, code="200"):
|
|
if "-o" in args:
|
|
pathlib.Path(args[args.index("-o") + 1]).write_text(body)
|
|
else:
|
|
sys.stdout.write(body)
|
|
if "-w" in args:
|
|
sys.stdout.write(code)
|
|
sys.exit(0)
|
|
|
|
# RECORD BEFORE FILTERING. This recorder used to live inside the `endswith` branch below, which
|
|
# made the "no ref reaches the URL" assertion unfalsifiable: the only URLs it could record were ones
|
|
# that already satisfied it, so a by-name request was invisible to the very test written to forbid
|
|
# it. Cold review reintroduced a by-name lookup in the hook and the suite stayed 33/33 green. That is
|
|
# the filter-on-the-asserted-property defect this PR's sibling record is about, committed inside the
|
|
# guard against it — so the recorder now sees EVERY branch-protection URL, whatever its shape.
|
|
if "/branch_protections" in url:
|
|
with (state / "bp_urls").open("a") as fh:
|
|
fh.write(url + "\n")
|
|
|
|
if url.rstrip("/").endswith("/branch_protections"):
|
|
# The hook reads ONLY this endpoint now — the by-name lookup was deleted because it performs no
|
|
# matching and knows nothing about rule precedence, so a 200 from it proved less than it looked.
|
|
mode = (state / "bp").read_text().strip()
|
|
|
|
if mode == "TRANSPORT-ERROR":
|
|
respond("", "000")
|
|
if mode == "FORBIDDEN":
|
|
respond('{"message":"token does not have at least one of required scope(s)"}', "403")
|
|
if mode == "GARBAGE":
|
|
respond('{"message":"not an array"}')
|
|
if mode == "EMPTY":
|
|
respond('')
|
|
if mode == "UNPARSEABLE-RULES":
|
|
# A 200 whose rule NAME is a number: `//` fires only on null/false, so the classifier's
|
|
# `explode`/`match` throws and the program dies on a read that plainly succeeded.
|
|
respond(json.dumps([{"branch_name": 7, "enable_status_check": True,
|
|
"status_check_contexts": ["review-verdict/h10"]}]))
|
|
if mode == "LIST-404":
|
|
# An HTTP 404 from the LIST endpoint: the repo is absent, or invisible to this credential.
|
|
# Gitea answers 404 for both, and it says NOTHING about whether the base is protected.
|
|
respond('{"message":"Not Found"}', "404")
|
|
if mode == "EMPTY-LIST":
|
|
# The list WAS read and holds no rule — the only shape that establishes absence.
|
|
respond("[]")
|
|
if mode == "LIST-UNREADABLE":
|
|
respond('{"message":"internal error"}', "500")
|
|
if mode == "GLOB-RULE":
|
|
respond(json.dumps([{"branch_name": "m*", "enable_status_check": True,
|
|
"status_check_contexts": ["review-verdict/h10"]}]))
|
|
if mode == "REGEX-META-RULE":
|
|
respond(json.dumps([{"branch_name": "mai.", "enable_status_check": True,
|
|
"status_check_contexts": ["review-verdict/h10"]}]))
|
|
if mode == "GLOB-WITH-DOT-RULE":
|
|
respond(json.dumps([{"branch_name": "release/26.*", "enable_status_check": True,
|
|
"status_check_contexts": ["review-verdict/h10"]}]))
|
|
if mode == "CHARCLASS-RULE":
|
|
respond(json.dumps([{"branch_name": "a[b", "enable_status_check": True,
|
|
"status_check_contexts": ["review-verdict/h10"]},
|
|
{"branch_name": "main", "enable_status_check": True,
|
|
"status_check_contexts": REQUIRED}]))
|
|
if mode == "ESCAPED-META-RULE":
|
|
respond(json.dumps([{"branch_name": "a\\{b", "enable_status_check": True,
|
|
"status_check_contexts": ["review-verdict/h10"]}]))
|
|
if mode == "DRIFTED-CONTEXTS":
|
|
# Still requires review-verdict/h10, so every check ABOVE the freshness one is satisfied and
|
|
# the hook reaches it — but the live list carries a context the committed snapshot does not.
|
|
respond(json.dumps([{"branch_name": "main", "enable_status_check": True,
|
|
"status_check_contexts":
|
|
[*REQUIRED, "Build ErsatzTV Image / Fourth (pull_request)"]}]))
|
|
if mode == "CASEFOLD-RULE":
|
|
respond(json.dumps([{"branch_name": "MAIN", "enable_status_check": True,
|
|
"status_check_contexts": REQUIRED}]))
|
|
if mode == "TWO-CASE-VARIANT-RULES":
|
|
respond(json.dumps([{"branch_name": "MAIN", "enable_status_check": True,
|
|
"status_check_contexts": ["review-verdict/h10"]},
|
|
{"branch_name": "main", "enable_status_check": True,
|
|
"status_check_contexts": ["Build ErsatzTV Image / Build & test (.NET)"]}]))
|
|
if mode == "NONASCII-RULE":
|
|
respond(json.dumps([{"branch_name": "\u00fcnstable", "enable_status_check": True,
|
|
"status_check_contexts": ["review-verdict/h10"]}]))
|
|
if mode == "EXACT-PLUS-GLOB-RULE":
|
|
respond(json.dumps([{"branch_name": "main", "enable_status_check": True,
|
|
"status_check_contexts": ["review-verdict/h10"]},
|
|
{"branch_name": "m*", "enable_status_check": True,
|
|
"status_check_contexts": ["Build ErsatzTV Image / Build & test (.NET)"]}]))
|
|
if mode == "MALFORMED-MEMBER":
|
|
respond(json.dumps([{"branch_name": "main", "enable_status_check": True,
|
|
"status_check_contexts": 7}]))
|
|
if mode == "SUBSTRING-STRING":
|
|
respond(json.dumps([{"branch_name": "main", "enable_status_check": True,
|
|
"status_check_contexts": "prefix-review-verdict/h10-suffix"}]))
|
|
if mode == "FALSE-CONTEXTS":
|
|
respond(json.dumps([{"branch_name": "main", "enable_status_check": True,
|
|
"status_check_contexts": False}]))
|
|
if mode == "STRING-ENABLE":
|
|
respond(json.dumps([{"branch_name": "main", "enable_status_check": "true",
|
|
"status_check_contexts": ["review-verdict/h10"]}]))
|
|
if mode == "NON-STRING-MEMBER":
|
|
respond(json.dumps([{"branch_name": "main", "enable_status_check": True,
|
|
"status_check_contexts": [7, "review-verdict/h10"]}]))
|
|
if mode == "EMPTY-CONTEXTS":
|
|
respond(json.dumps([{"branch_name": "main", "enable_status_check": True,
|
|
"status_check_contexts": []}]))
|
|
if mode == "NULL-CONTEXTS":
|
|
respond(json.dumps([{"branch_name": "main", "enable_status_check": True,
|
|
"status_check_contexts": None}]))
|
|
if mode == "LONGER-STRING-MEMBER":
|
|
respond(json.dumps([{"branch_name": "main", "enable_status_check": True,
|
|
"status_check_contexts": ["xx-review-verdict/h10-yy"]}]))
|
|
if mode == "STATUS-CHECK-OFF":
|
|
respond(json.dumps([{"branch_name": "main", "enable_status_check": False,
|
|
"status_check_contexts": ["review-verdict/h10"]}]))
|
|
if mode == "MISSING-CONTEXT":
|
|
respond(json.dumps([{"branch_name": "main", "enable_status_check": True,
|
|
"status_check_contexts":
|
|
["Build ErsatzTV Image / Build & test (.NET) (pull_request)"]}]))
|
|
# GUARDED
|
|
respond(json.dumps([{"branch_name": "main", "enable_status_check": True,
|
|
"status_check_contexts": REQUIRED}]))
|
|
|
|
if "/pulls/" in url and "/files" in url:
|
|
q = urllib.parse.parse_qs(urllib.parse.urlparse(url).query)
|
|
page = int(q.get("page", ["1"])[0])
|
|
if page == 1:
|
|
print(json.dumps([{"filename": "ErsatzTV/Program.cs", "status": "modified"}]))
|
|
else:
|
|
print("[]")
|
|
sys.exit(0)
|
|
|
|
if "/status" in url and (state / "no_recorded_base").exists():
|
|
# A verdict posted before ersatztv#632 carries no `(base: …)` marker, so the #632 detection
|
|
# takes its graceful-adoption path and forms no opinion. That isolates the hoisted retarget
|
|
# check as the ONLY guard that can deny.
|
|
print(json.dumps({"state": "success", "statuses": [
|
|
{"context": "review-verdict/h10", "status": "success",
|
|
"description": "Review-verdict: MERGEABLE @ %s" % os.environ["STUB_SHORT"]}]}))
|
|
sys.exit(0)
|
|
|
|
if "/status" in url:
|
|
# A 2xx body whose `.statuses` IS an array but whose members are scalars. `.statuses | type ==
|
|
# "array"` passes; indexing a number then makes jq exit 5 and, under `set -e`, kills the hook
|
|
# with no JSON at all.
|
|
_m = (state / "bp").read_text().strip()
|
|
if _m == "SCALAR-STATUS-ROW":
|
|
print('{"state":"success","statuses":[1]}')
|
|
sys.exit(0)
|
|
if _m == "NONSTRING-STATUS-ROW":
|
|
# Object shape, string context, valid description — passes the #632 block, which does NOT
|
|
# validate `.status` — but a NUMERIC status. This is the shape that actually reaches the
|
|
# scheduled branch's validator, i.e. the new clause's reachable contribution.
|
|
print(json.dumps({"state": "success", "statuses": [
|
|
{"context": "review-verdict/h10", "status": 7,
|
|
"description": "Review-verdict: MERGEABLE @ %s (base: main)"
|
|
% os.environ["STUB_SHORT"]}]}))
|
|
sys.exit(0)
|
|
print(json.dumps({"state": "success", "statuses": [
|
|
{"context": "review-verdict/h10", "status": "success",
|
|
"description": "Review-verdict: MERGEABLE @ %s (base: main)" % os.environ["STUB_SHORT"]}]}))
|
|
sys.exit(0)
|
|
|
|
if "/issues/" in url and "/comments" in url:
|
|
print(json.dumps([{"body": "Review-verdict: MERGEABLE @ %s" % os.environ["STUB_SHORT"]}]))
|
|
sys.exit(0)
|
|
|
|
if "/issues/" in url:
|
|
print(json.dumps({"body": "## Done-when\n- [x] everything\n"}))
|
|
sys.exit(0)
|
|
|
|
if "/pulls/" in url:
|
|
# The base is served per-GET so a PERSISTENT retarget mid-run can be modelled: the first read
|
|
# (top of the hook) sees `main`, a later one sees whatever `retarget` names.
|
|
counter = state / "pr_get_count"
|
|
n = int(counter.read_text()) if counter.exists() else 0
|
|
counter.write_text(str(n + 1))
|
|
base = "main"
|
|
bo = state / "base_override"
|
|
if bo.exists():
|
|
base = bo.read_text().strip()
|
|
rt = state / "retarget"
|
|
if rt.exists() and n >= 1:
|
|
base = rt.read_text().strip()
|
|
print(json.dumps({"head": {"sha": os.environ["STUB_SHA"]},
|
|
"base": {"ref": base, "sha": "b" * 40},
|
|
"body": "fixes #1"}))
|
|
sys.exit(0)
|
|
|
|
print("{}")
|
|
"""
|
|
|
|
|
|
@pytest.fixture
|
|
def hook(tmp_path):
|
|
bindir = tmp_path / "bin"
|
|
bindir.mkdir()
|
|
curl = bindir / "curl"
|
|
curl.write_text(CURL_SHIM)
|
|
curl.chmod(0o755)
|
|
state = tmp_path / "state"
|
|
state.mkdir()
|
|
(state / "bp").write_text("GUARDED")
|
|
|
|
env = dict(os.environ)
|
|
env["PATH"] = f"{bindir}{os.pathsep}{env['PATH']}"
|
|
env["STUB_DIR"] = str(state)
|
|
env["REQUIRED_CONTEXTS_JSON"] = str(REPO_ROOT / ".gitea" / "required-status-contexts.json")
|
|
env["STUB_SHA"] = SHA
|
|
env["STUB_SHORT"] = SHORT
|
|
env["ETV_GITEA_TOKEN"] = "stub" # noqa: S105 - deliberately fake; the real credential comes from the environment
|
|
env["ETV_GITEA_URL"] = "http://gitea.example"
|
|
env["CLAUDE_PROJECT_DIR"] = str(REPO_ROOT)
|
|
env.pop("ETV_GITEA_BASICAUTH", None)
|
|
|
|
class Handle:
|
|
def set_branch_protection(self, mode):
|
|
(state / "bp").write_text(mode)
|
|
|
|
def set_base(self, ref):
|
|
"""The base reported by EVERY PR read — a stable target, not a retarget."""
|
|
(state / "base_override").write_text(ref)
|
|
|
|
def drop_recorded_base(self):
|
|
"""Serve a pre-#632 verdict (no `(base: …)`), so only the hoisted check can deny."""
|
|
(state / "no_recorded_base").write_text("1")
|
|
|
|
def set_retarget(self, ref):
|
|
"""Persistent retarget: every PR read after the first reports `ref`."""
|
|
(state / "retarget").write_text(ref)
|
|
|
|
def branch_protection_urls(self):
|
|
f = state / "bp_urls"
|
|
return f.read_text().splitlines() if f.exists() else []
|
|
|
|
def decision(self, scheduled=True, project_dir=None, hook_path=None, owner="timothy", repo="ersatztv"):
|
|
payload = {
|
|
"tool_input": {
|
|
"method": "merge",
|
|
"owner": owner,
|
|
"repo": repo,
|
|
"pull_number": 42,
|
|
"merge_when_checks_succeed": scheduled,
|
|
}
|
|
}
|
|
run_env = env if project_dir is None else {**env, "CLAUDE_PROJECT_DIR": project_dir}
|
|
r = subprocess.run(
|
|
["bash", str(hook_path or HOOK)],
|
|
input=json.dumps(payload),
|
|
env=run_env,
|
|
capture_output=True,
|
|
text=True,
|
|
)
|
|
assert r.returncode == 0, r.stderr
|
|
if not r.stdout.strip():
|
|
return None
|
|
return json.loads(r.stdout)
|
|
|
|
def reason(self, scheduled=True):
|
|
d = self.decision(scheduled=scheduled)
|
|
return "" if d is None else json.dumps(d)
|
|
|
|
return Handle()
|
|
|
|
|
|
def test_a_base_without_the_required_check_denies_a_SCHEDULED_merge(hook):
|
|
"""The defect #778 closes: arming an auto-merge while the per-sha gate that makes it safe is
|
|
absent. Nothing else in the flow notices, which is what makes it worth a guard."""
|
|
hook.set_branch_protection("MISSING-CONTEXT")
|
|
reason = hook.reason()
|
|
assert "deny" in reason, (
|
|
"a scheduled auto-merge was armed with no 'review-verdict/h10' required check on the base — "
|
|
"that is ersatztv#622's hole reopened"
|
|
)
|
|
assert "review-verdict/h10" in reason, (
|
|
"the deny must name the missing context; a reader cannot act on 'branch protection is wrong'"
|
|
)
|
|
|
|
|
|
def test_status_checks_disabled_wholesale_also_denies(hook):
|
|
"""The context can be listed while `enable_status_check` is false, in which case Gitea enforces
|
|
none of them. Reading only the list would report the protection as present — the same
|
|
check-the-label-not-the-capability shape (#697/#698) this repo has paid for twice."""
|
|
hook.set_branch_protection("STATUS-CHECK-OFF")
|
|
reason = hook.reason()
|
|
assert "deny" in reason, "status checks were disabled entirely and the listed context was read as protection anyway"
|
|
|
|
|
|
def test_positive_control_a_guarded_base_REACHES_the_check_and_still_auto_grants(hook):
|
|
"""Without this, every test above passes if the hook denies on all paths — which it very nearly
|
|
does, since this PR is non-docs and several later conditions are stubbed only loosely.
|
|
|
|
Asserting the absence of one phrase was not enough (cold review): an unrelated early `ask`, or a
|
|
differently-worded deny, would satisfy it while proving nothing. So this pins all three of the
|
|
things that must be true — the branch-protection endpoint was actually CALLED, the decision is
|
|
`allow`, and the reason is the satisfied-gate message rather than any refusal.
|
|
"""
|
|
decision = hook.decision()
|
|
assert hook.branch_protection_urls(), (
|
|
"the guarded case never reached the branch-protection endpoint, so the other tests are not "
|
|
"exercising the code they claim to"
|
|
)
|
|
assert decision is not None, "the hook passed through instead of auto-granting"
|
|
verdict = decision["hookSpecificOutput"]["permissionDecision"]
|
|
assert verdict == "allow", f"a fully-satisfied gate did not auto-grant (got {verdict!r})"
|
|
assert "satisfied" in decision["hookSpecificOutput"]["permissionDecisionReason"]
|
|
|
|
|
|
@pytest.mark.parametrize("shape", ["SUBSTRING-STRING", "LONGER-STRING-MEMBER"])
|
|
def test_a_context_name_that_merely_CONTAINS_the_required_one_does_not_satisfy_it(hook, shape):
|
|
"""The false-OPEN this guard must not have.
|
|
|
|
jq's `index()` on a STRING is substring search, so `"prefix-review-verdict/h10-suffix"` answers
|
|
yes to a naive membership test — auto-granting a scheduled merge on a base where the context is
|
|
not required at all. `LONGER-STRING-MEMBER` covers the same confusion inside a real array.
|
|
A false-closed here costs one prompt; a false-open costs an unreviewed merge, so the membership
|
|
test is exact equality over a value first proven to be an array of strings.
|
|
"""
|
|
hook.set_branch_protection(shape)
|
|
reason = hook.reason()
|
|
assert "allow" not in reason or "deny" in reason or "ask" in reason, (
|
|
f"payload shape {shape} auto-granted a scheduled merge"
|
|
)
|
|
assert "satisfied" not in reason, (
|
|
f"a context name that merely contains 'review-verdict/h10' ({shape}) was accepted as it"
|
|
)
|
|
|
|
|
|
@pytest.mark.parametrize("shape", ["EMPTY-CONTEXTS", "NULL-CONTEXTS"])
|
|
def test_an_empty_or_null_contexts_list_DENIES_rather_than_asking(hook, shape):
|
|
"""Absent is the finding, not a read failure. An empty or null list is a well-formed answer
|
|
meaning "nothing is required here", so it must take the deny arm and not be swept into the
|
|
unknown-shape ask alongside genuinely unreadable payloads."""
|
|
hook.set_branch_protection(shape)
|
|
reason = hook.reason()
|
|
assert "deny" in reason, f"{shape} was treated as unreadable rather than as a confirmed absent required check"
|
|
|
|
|
|
def test_a_FALSE_contexts_value_asks_rather_than_denying(hook):
|
|
"""jq's `//` alternative fires on `false`, not only on null, so `// []` mapped this malformed
|
|
payload to an empty list and answered "no" — a confident deny derived from a shape that was
|
|
never understood. Absent and null are defaulted explicitly; everything else is unknown."""
|
|
hook.set_branch_protection("FALSE-CONTEXTS")
|
|
reason = hook.reason()
|
|
assert "ask" in reason, "a false contexts value was defaulted to [] and produced a deny"
|
|
|
|
|
|
def test_a_non_string_MEMBER_inside_the_array_asks(hook):
|
|
"""`[7, "review-verdict/h10"]` contains the context, but the payload is not the shape this
|
|
guard knows how to reason about. Answering "yes" would mean trusting a structure we cannot
|
|
validate; the honest answer is that we could not tell."""
|
|
hook.set_branch_protection("NON-STRING-MEMBER")
|
|
reason = hook.reason()
|
|
assert "ask" in reason, "an array with a non-string member produced a decision anyway"
|
|
|
|
|
|
def test_a_malformed_contexts_MEMBER_asks_rather_than_denying_with_the_wrong_reason(hook):
|
|
"""One level below the response-shape check, and it survives it.
|
|
|
|
`{"status_check_contexts": 7}` is a perfectly good object, so the top-level type guard passes;
|
|
jq then errors on the member, `|| true` turns that into an empty string, and a two-way test
|
|
would report "NOT a required status check" — a confident, specific, wrong diagnosis of a payload
|
|
that was never read. The same swallow one level down as the #632 base-change guard's second fix.
|
|
"""
|
|
hook.set_branch_protection("MALFORMED-MEMBER")
|
|
reason = hook.reason()
|
|
assert "ask" in reason, "a malformed contexts member produced a decision instead of a question"
|
|
assert "NOT a required status check" not in reason, (
|
|
"an unreadable payload was reported as a confirmed missing required check"
|
|
)
|
|
|
|
|
|
def test_a_base_with_NO_branch_protection_at_all_denies_rather_than_asking(hook):
|
|
"""The strongest form of the thing being checked, and the likeliest real trigger.
|
|
|
|
A rule list that is READABLE and EMPTY has nothing that can govern any base, so
|
|
`review-verdict/h10` is definitively not required and scheduling an auto-merge is #622's hole.
|
|
That must DENY, not ask: routing the most likely real-world trigger — branch protection removed
|
|
— to a human prompt would make it read like a transient hiccup.
|
|
|
|
Absence is established by the LIST, never by a status code; this fixture returns 200 with `[]`.
|
|
An HTTP 404 means the repo was absent or invisible to the credential and is a read failure,
|
|
covered by `test_an_HTTP_404_on_the_LIST_read_asks_and_does_not_claim_the_list_was_read`.
|
|
"""
|
|
hook.set_branch_protection("EMPTY-LIST")
|
|
reason = hook.reason()
|
|
assert "deny" in reason, "a base with no branch protection at all did not deny a scheduled auto-merge"
|
|
assert "none matches" in reason, (
|
|
"the deny must distinguish 'the list was read and nothing governs this base' from 'could not read'"
|
|
)
|
|
|
|
|
|
def test_a_403_asks_because_it_says_only_that_we_could_not_look(hook):
|
|
"""A credential without the repo-admin scope this endpoint needs proves nothing about the
|
|
protection, so it must NOT deny — otherwise the guard strands every scheduled merge run made
|
|
with a narrower token."""
|
|
hook.set_branch_protection("FORBIDDEN")
|
|
reason = hook.reason()
|
|
assert "ask" in reason, "a 403 was treated as evidence about the protection"
|
|
assert "none matches" not in reason, "a 403 was reported as a confirmed absence of branch protection"
|
|
|
|
|
|
@pytest.mark.parametrize("failure", ["TRANSPORT-ERROR", "GARBAGE", "EMPTY", "LIST-404"])
|
|
def test_an_UNREADABLE_branch_protection_asks_rather_than_denying_or_passing(hook, failure):
|
|
""" "Could not check" is a third outcome, not a synonym for either neighbour.
|
|
|
|
Denying would strand every scheduled merge on a Gitea hiccup or on credentials without the
|
|
repo-admin scope this endpoint needs. Passing would be worse: it would restore the exact
|
|
unverified assumption #778 exists to remove, while now printing a reason string claiming the
|
|
protection was confirmed.
|
|
"""
|
|
hook.set_branch_protection(failure)
|
|
reason = hook.reason()
|
|
assert "ask" in reason, f"an unreadable branch-protection response ({failure}) did not fall through to a human"
|
|
assert "branch-protection rules" in reason, "the ask must name what could not be checked"
|
|
|
|
|
|
def test_an_IMMEDIATE_merge_is_not_subjected_to_this_check(hook):
|
|
"""Scope, deliberately narrow — and stated without the overclaim cold review removed.
|
|
|
|
An immediate merge is not window-FREE: the hook returns `allow` and a separate call performs the
|
|
merge, so a push can still land in between. What it lacks is a SCHEDULER — nothing waits on
|
|
pending checks, so the gap is one tool call rather than however long CI takes. The required
|
|
branch-protection check is what protects the scheduled path specifically, so extending this deny
|
|
to immediate merges would block a materially safer operation and invite the whole guard being
|
|
switched off. The residual on this path is carried in docs/remote-state-inventory.md.
|
|
"""
|
|
hook.set_branch_protection("MISSING-CONTEXT")
|
|
reason = hook.reason(scheduled=False)
|
|
assert "NOT a required status check" not in reason, (
|
|
"the required-check deny fired on an immediate merge, which has no post-scheduling window"
|
|
)
|
|
|
|
|
|
def test_a_NON_BOOLEAN_enable_status_check_asks(hook):
|
|
"""`"true"` is not `true`. Comparing the string to `true` yields a confident "no" -> deny from a
|
|
payload never understood, which collapses the documented tri-state into two states. Every
|
|
malformed shape on this endpoint has to reach the same ask arm."""
|
|
hook.set_branch_protection("STRING-ENABLE")
|
|
reason = hook.reason()
|
|
assert "ask" in reason, "a string enable_status_check produced a decision instead of a question"
|
|
assert "NOT a required status check" not in reason
|
|
|
|
|
|
def test_a_SCALAR_status_row_asks_instead_of_killing_the_hook(hook):
|
|
"""The consent hook's contract is that it always emits exactly one of grant/deny/ask, and
|
|
`{"statuses":[1]}` is the payload that can break it: it passes an `.statuses | type == "array"`
|
|
check, after which indexing a number errors and exits 5, which under `set -e` aborts the hook
|
|
with NO JSON at all. A gate that emits nothing has not failed closed; it has failed to decide.
|
|
|
|
WHAT THIS TEST DOES *NOT* PROVE, stated because the mutation showed it. Restoring the
|
|
scheduled-branch validation to its predecessor leaves this test GREEN, because the #632
|
|
base-retarget block runs FIRST and validates the members it consumes — so it catches THIS
|
|
payload and asks before the scheduled branch is reached. The two guards overlap, which is the
|
|
masking shape `duplicate guards mask each other` describes.
|
|
|
|
The caveat is scoped to this payload, NOT to the clause. The earlier block validates `.context`
|
|
and `.description` but not `.status`, so an object row with a numeric status passes it and does
|
|
reach the new validator — `test_a_NON_STRING_status_reaches_the_scheduled_validator` covers that
|
|
and goes red when the clause is removed. So the clause is masked for scalar rows and load-bearing
|
|
for that one. An earlier draft called the whole clause defence-in-depth, understating it in the
|
|
opposite direction from this repo's usual error.
|
|
|
|
So this asserts the OBSERVABLE contract — a decision is always emitted for this payload — which
|
|
is true and worth pinning whichever guard supplies it. It is deliberately not offered as a
|
|
mutation proof of the newer clause, because it is not one.
|
|
"""
|
|
hook.set_branch_protection("SCALAR-STATUS-ROW")
|
|
decision = hook.decision()
|
|
assert decision is not None, (
|
|
"the hook emitted no decision at all for a malformed statuses payload — it neither granted, denied nor asked"
|
|
)
|
|
assert decision["hookSpecificOutput"]["permissionDecision"] == "ask"
|
|
|
|
|
|
def test_a_PERSISTENT_retarget_denies_on_the_IMMEDIATE_path_too(hook):
|
|
"""The twin. The re-read first landed inside the scheduled branch only, so this exact case —
|
|
same fixture, `merge_when_checks_succeed` absent — AUTO-GRANTED while its sibling denied.
|
|
|
|
Cold review demonstrated it side by side, and it is the shape this repo has on record as
|
|
"fix one path, then check its TWIN": the fix was applied where the defect was noticed, and the
|
|
other consumer of the same stale value kept it. The re-read is now hoisted above every
|
|
base-dependent decision rather than duplicated into the branch that happened to be under review.
|
|
"""
|
|
hook.drop_recorded_base()
|
|
hook.set_retarget("scratch")
|
|
reason = hook.reason(scheduled=False)
|
|
assert "deny" in reason, "an immediate merge was auto-granted after the PR was retargeted mid-evaluation"
|
|
assert "scratch" in reason and "main" in reason
|
|
|
|
|
|
def test_a_PERSISTENT_retarget_after_the_first_read_denies(hook):
|
|
"""The defect this guard had itself, found in the fifth cold-review round.
|
|
|
|
`$base_ref` is captured from the PR snapshot at the top of the hook, and everything between
|
|
then and the branch-protection lookup is round trips — the file enumeration alone can be forty
|
|
pages. A retarget in that gap needs no ABA and no force-push: the lookup would name the OLD
|
|
base, confirm `review-verdict/h10` on a branch the PR no longer targets, and grant a scheduled
|
|
merge onto one that may require nothing. Checking a stale identifier is not checking, which is
|
|
the whole of `process.check-and-use-pins-a-version` — so the guard enforcing that rule had to
|
|
stop breaking it.
|
|
"""
|
|
hook.drop_recorded_base()
|
|
hook.set_retarget("scratch")
|
|
reason = hook.reason()
|
|
assert "deny" in reason, "the PR was retargeted mid-evaluation and the gate still granted on the original base"
|
|
assert "scratch" in reason and "main" in reason, (
|
|
"the deny must name both branches; a reader cannot act on 'the base changed'"
|
|
)
|
|
|
|
|
|
def test_a_NON_STRING_status_reaches_the_scheduled_validator(hook):
|
|
"""The reachable contribution of the scheduled-branch member validation, which the previous
|
|
caveat understated.
|
|
|
|
The #632 block validates `.context` and `.description` but NOT `.status`, so an object row with
|
|
a numeric status passes it and arrives here. Without this clause it becomes `vstate=7` and falls
|
|
to the catch-all deny arm — fail-closed, but reported as "the verdict is '7'" rather than as a
|
|
payload that could not be read. So the clause is masked for scalar rows and load-bearing for
|
|
this one; the caveat on the scalar test is scoped accordingly.
|
|
"""
|
|
hook.set_branch_protection("NONSTRING-STATUS-ROW")
|
|
reason = hook.reason()
|
|
assert "ask" in reason, "a non-string .status produced a verdict-shaped decision"
|
|
|
|
|
|
def test_a_base_that_a_GLOB_rule_could_govern_ASKS(hook):
|
|
"""A base covered only by a glob rule has no rule bearing its own name, and the deleted by-name
|
|
endpoint answered 404 for exactly that — read as "unprotected", producing a hard DENY with a
|
|
specific, false cause.
|
|
|
|
But the opposite error is worse: deciding the glob DOES match would auto-grant on a base whose
|
|
protection was never established. This hook does not reimplement Gitea's glob dialect (its `*`
|
|
does not cross `/`, and `?`/`[…]`/`{a,b}` are wildcards), so a glob rule that COULD govern the
|
|
base is undecidable and asks — the only answer honest in both directions.
|
|
"""
|
|
hook.set_branch_protection("GLOB-RULE")
|
|
reason = hook.reason()
|
|
assert "ask" in reason, "a base a glob rule could govern was decided rather than referred to a human"
|
|
assert "could govern it" in reason, (
|
|
"the ask came from the generic could-not-read arm, not the undecidable-glob arm — those "
|
|
"are different outcomes and a crashed classifier must not pass as a correct classification"
|
|
)
|
|
assert "none matches" not in reason, "a base a glob rule could govern was reported as having no protection at all"
|
|
|
|
|
|
def test_an_unreadable_rule_LIST_asks_rather_than_denying(hook):
|
|
"""A read failure must not convert 'could not confirm' into 'confirmed absent'. Absence is
|
|
established only by the classifier returning `nomatch` over a list that WAS read."""
|
|
hook.set_branch_protection("LIST-UNREADABLE")
|
|
reason = hook.reason()
|
|
assert "ask" in reason, "an unreadable rule list was treated as proof of absence"
|
|
assert "none matches" not in reason
|
|
|
|
|
|
def test_the_protection_lookup_puts_NO_ref_in_the_url(hook):
|
|
"""The successor to a URL-encoding test, and the reason it could be retired.
|
|
|
|
The ref used to be interpolated into `branch_protections/{name}`, where a base like
|
|
`release/26.4` injected a path separator and 404'd — read as "unprotected". That endpoint is
|
|
gone: it performed no matching and knew nothing about rule precedence, so a 200 from it proved
|
|
less than it looked. Only the LIST endpoint is read now, which takes no ref at all, so the whole
|
|
encoding hazard is removed by construction rather than escaped.
|
|
"""
|
|
hook.set_base("release/26.4")
|
|
hook.drop_recorded_base()
|
|
hook.reason()
|
|
urls = hook.branch_protection_urls()
|
|
assert urls, "the branch-protection endpoint was never requested"
|
|
for u in urls:
|
|
assert u.rstrip("/").endswith("/branch_protections"), f"a ref reached the branch-protection URL: {u}"
|
|
|
|
|
|
def test_a_rule_name_with_REGEX_METACHARACTERS_does_not_match_a_different_base(hook):
|
|
"""The false-open in the glob fallback: `*` must be the only wildcard.
|
|
|
|
Substituting `*` into a raw regex left every other metacharacter live, so a rule named `mai.`
|
|
matched the base `main` (and `a+b` matched `aab`). A spurious match to some OTHER rule that
|
|
happens to require `review-verdict/h10` reports this base as protected when nothing governs it —
|
|
a consent gate answering yes on evidence about a different branch. Verified directly before the
|
|
fix: `main.x` matched `mainax`.
|
|
|
|
Here the only rule is `mai.`, which governs a branch that is not `main`, so nothing protects the
|
|
base and the gate must deny rather than grant.
|
|
"""
|
|
hook.set_branch_protection("REGEX-META-RULE")
|
|
reason = hook.reason()
|
|
assert "deny" in reason, "a rule named 'mai.' was regex-matched against base 'main' and read as protection"
|
|
assert "none matches" in reason, (
|
|
"'mai.' contains no GLOB metacharacter, so it is decidable: it simply does not govern "
|
|
"'main', and the base is genuinely unprotected"
|
|
)
|
|
|
|
|
|
def test_a_GLOB_rule_whose_literal_part_has_a_metacharacter_still_MATCHES(hook):
|
|
"""The positive control the first escaping attempt lacked, and the reason it looked green.
|
|
|
|
Escaping is only half the property: `*` must still span. The first version emitted TWO
|
|
backslashes (`\\.` = "a literal backslash, then any character"), which made every rule
|
|
containing a metacharacter UNMATCHABLE — so the fallback found nothing and hard-denied with the
|
|
stated cause that no rule can govern the base — a false-open converted into a false DENY.
|
|
|
|
A negative-only assertion cannot see that: a rule matched literally and a rule made unmatchable
|
|
both fail to match the wrong base. Only a rule that SHOULD match distinguishes them. Here the rule
|
|
`release/26.*` could govern the base `release/26.4`, so the gate must reach a decision about it
|
|
rather than reporting the base as unprotected.
|
|
"""
|
|
hook.set_base("release/26.4")
|
|
hook.drop_recorded_base() # keep the #632 comparison out of this test's way
|
|
hook.set_branch_protection("GLOB-WITH-DOT-RULE")
|
|
reason = hook.reason()
|
|
assert "ask" in reason, (
|
|
"the glob rule 'release/26.*' could govern base 'release/26.4', which is undecidable here and must ask"
|
|
)
|
|
assert "could govern it" in reason, (
|
|
"the ask must come from the undecidable-glob arm, not from the classifier failing"
|
|
)
|
|
assert "none matches" not in reason, (
|
|
"a base a glob rule could govern was reported as entirely unprotected — the over-escaping "
|
|
"failure this test exists to catch"
|
|
)
|
|
|
|
|
|
def test_a_rule_name_containing_a_CHAR_CLASS_bracket_does_not_crash_the_matcher(hook):
|
|
"""`a[b` built the pattern `a\\[b`, which is a premature end of char-class: jq exits 5, the
|
|
`|| true` swallows it, and the whole list is discarded — so an unrelated rule's NAME could
|
|
poison the lookup and deny a base that a later rule in the same list protects."""
|
|
hook.set_branch_protection("CHARCLASS-RULE")
|
|
reason = hook.reason()
|
|
assert reason, "the hook emitted no decision at all"
|
|
assert "ask" not in reason, (
|
|
"the exact-name rule was decidable and must have been honoured; an ask here means the "
|
|
"classifier failed rather than classified"
|
|
)
|
|
assert "none matches" not in reason, (
|
|
"one rule with a bracket in its name discarded the whole list, including the exact-name "
|
|
"rule that actually protects this base"
|
|
)
|
|
assert "deny" not in reason, "an exact-name rule requiring review-verdict/h10 was present and was not honoured"
|
|
|
|
|
|
def test_a_plain_rule_name_is_matched_CASE_INSENSITIVELY(hook):
|
|
"""Gitea compares a rule name with no glob metacharacter using `EqualFold`, so a rule named
|
|
`MAIN` governs the base `main`. Comparing case-sensitively here would find no rule, conclude the
|
|
base is unprotected, and deny with a false stated cause."""
|
|
hook.set_branch_protection("CASEFOLD-RULE")
|
|
reason = hook.reason()
|
|
assert "ask" not in reason, (
|
|
"the case-folded exact rule was decidable and must have been honoured; an ask means the "
|
|
"classifier failed rather than classified"
|
|
)
|
|
assert "none matches" not in reason, (
|
|
"a rule named 'MAIN' governs base 'main' in Gitea but was missed by a case-sensitive compare"
|
|
)
|
|
assert "deny" not in reason
|
|
|
|
|
|
def test_a_BACKSLASH_ESCAPED_metacharacter_in_a_rule_name_is_undecidable_not_absent(hook):
|
|
"""The one case that breaks the superset proof the `none` arm rests on.
|
|
|
|
`none` authorises a DENY on the stated grounds that nothing can possibly govern this base, so
|
|
its premise must hold unconditionally. gobwas/glob reads `\\{` as a LITERAL brace, so the rule
|
|
`a\\{b` governs the base `a{b`; a superset that treated `\\` as an ordinary character would build
|
|
`a\\.*b`, fail to match, and deny a base that is in fact protected. Treating backslash as a
|
|
metacharacter restores the property.
|
|
|
|
Git ref rules make this nearly unreachable — a branch name may not contain `*`, `?`, `[` or `\\`
|
|
— but `{` IS legal in a branch name, and "nearly unreachable" is not the standard for the arm
|
|
that issues a deny.
|
|
"""
|
|
hook.set_base("a{b")
|
|
hook.drop_recorded_base()
|
|
hook.set_branch_protection("ESCAPED-META-RULE")
|
|
reason = hook.reason()
|
|
assert "none matches" not in reason, (
|
|
"a rule whose escaped brace governs this base was reported as unable to govern it"
|
|
)
|
|
assert "ask" in reason, "an escaped-metacharacter rule is undecidable here and must ask"
|
|
assert "could govern it" in reason, (
|
|
"the ask must come from the undecidable-glob arm, not from the classifier failing — this "
|
|
"test hits the same arm as its two siblings and needs the same pin"
|
|
)
|
|
|
|
|
|
def test_the_precedence_check_runs_even_when_an_exactly_named_rule_EXISTS(hook):
|
|
"""The twin the restructure deletes, pinned so it cannot come back.
|
|
|
|
The hook used to look the rule up by NAME first and only enumerate the list on a 404. That
|
|
by-name endpoint is an exact DB lookup that performs no matching and knows nothing about
|
|
precedence, so on a 200 — the path this repo actually takes, since its rule IS named `main` —
|
|
the gate granted having consulted one rule and never asked which rule Gitea would apply. The
|
|
precedence argument guarded the 404 path only: hardened code that was dead, next to live code
|
|
that was not.
|
|
|
|
`EXACT-PLUS-GLOB-RULE` is exactly that configuration: a rule NAMED `main` that requires
|
|
`review-verdict/h10`, plus `m*` that does not. Under the old flow the by-name hit returned the
|
|
`main` rule, saw h10 and granted. Now there is one path, so the classifier sees both rules and
|
|
refuses to guess which one Gitea applies.
|
|
"""
|
|
hook.set_branch_protection("EXACT-PLUS-GLOB-RULE")
|
|
reason = hook.reason()
|
|
assert "ask" in reason, "an exactly-named rule was trusted without asking which rule Gitea would actually apply"
|
|
assert "could govern it" in reason
|
|
# And the by-name endpoint must not be consulted at all — its existence is what split the paths.
|
|
for u in hook.branch_protection_urls():
|
|
assert u.rstrip("/").endswith("/branch_protections"), (
|
|
f"the by-name lookup is back, and with it the unguarded path: {u}"
|
|
)
|
|
|
|
|
|
def test_a_GLOB_rule_that_could_outrank_an_exact_one_wins_and_ASKS(hook):
|
|
"""The arm ORDER, pinned. Without this the reorder is invisible to the suite — swapping the arms
|
|
back left all 29 tests green, which is how an unproven change ships.
|
|
|
|
Gitea picks the governing rule with `GetFirstMatched` over a list sorted by Priority and THEN by
|
|
plain-name-ness, so a glob rule can outrank an exactly-named one. Here `main` requires
|
|
`review-verdict/h10` and `m*` does not. Evaluating `exact` first inspects the rule that requires
|
|
h10, concludes the base is protected, and AUTO-GRANTS a scheduled merge onto a base where the
|
|
check may not be enforced at all — #622's hole, reached through the block written to close it.
|
|
|
|
Evaluating `undecidable` first is sound without knowing Gitea's precedence rules, which is the
|
|
only claim this code is entitled to make about somebody else's resolver.
|
|
"""
|
|
hook.set_branch_protection("EXACT-PLUS-GLOB-RULE")
|
|
reason = hook.reason()
|
|
assert "ask" in reason, (
|
|
"an exact rule was trusted while a glob rule could outrank it — the gate granted on a base "
|
|
"whose enforced rule it never identified"
|
|
)
|
|
assert "could govern it" in reason, (
|
|
"the ask must come from the undecidable-glob arm, not from the classifier failing"
|
|
)
|
|
|
|
|
|
def test_two_rules_differing_only_in_CASE_are_undecidable(hook):
|
|
"""`first` picks list order; Gitea picks by Priority. With `MAIN` requiring `review-verdict/h10`
|
|
and `main` not, inspecting whichever the API happened to list first would auto-grant on a base
|
|
whose enforced rule was never identified — the same defect as the arm order, one level down."""
|
|
hook.set_branch_protection("TWO-CASE-VARIANT-RULES")
|
|
reason = hook.reason()
|
|
assert "ask" in reason, "two fold-equal rules disagree about review-verdict/h10 and one was picked by list order"
|
|
assert "could govern it" in reason
|
|
|
|
|
|
def test_a_NON_ASCII_rule_or_base_is_undecidable_rather_than_fold_compared(hook):
|
|
"""`ascii_downcase` is not Gitea's Unicode-aware `EqualFold`, so a rule `ünstable` and a base
|
|
`Ünstable` fold equal there and not here. The miss lands on `none`, which DENIES with the stated
|
|
cause "none matches" — and the backslash arm already rejects "nearly unreachable"
|
|
as a standard for the arm that issues a deny, so the same standard applies here."""
|
|
hook.set_base("\u00dcnstable")
|
|
hook.drop_recorded_base()
|
|
hook.set_branch_protection("NONASCII-RULE")
|
|
reason = hook.reason()
|
|
assert "none matches" not in reason, (
|
|
"a rule that folds equal to this base under EqualFold was reported as unable to govern it"
|
|
)
|
|
assert "ask" in reason
|
|
|
|
|
|
def test_an_HTTP_404_on_the_LIST_read_asks_and_does_not_claim_the_list_was_read(hook):
|
|
"""The `nomatch` sentinel, pinned — it shipped UNPINNED, and a full revert left the suite green.
|
|
|
|
Absence must be established by the CLASSIFIER over a list that was actually read, never by an
|
|
HTTP status. Gitea answers 404 on this endpoint when the repo is absent or invisible to the
|
|
credential, which says nothing about the base. Reusing 404 for the classifier's own
|
|
nothing-can-govern verdict let that read reach the deny whose reason states "the full rule list
|
|
was read and none matches" — a claim about a read that never happened.
|
|
|
|
No fixture emitted an HTTP 404 on the list before this test, which is exactly why reverting the
|
|
sentinel to `bp_code=404` changed nothing observable.
|
|
"""
|
|
hook.set_branch_protection("LIST-404")
|
|
reason = hook.reason()
|
|
assert "ask" in reason, "an unreadable repo was treated as evidence about the base"
|
|
assert "none matches" not in reason, "a 404 read claimed the full rule list had been read and matched nothing"
|
|
|
|
|
|
@pytest.mark.parametrize("shape", ["UNPARSEABLE-RULES", "GARBAGE", "EMPTY"])
|
|
def test_a_200_the_classifier_cannot_PARSE_asks_without_blaming_the_transport(hook, shape):
|
|
"""The twin of the `nomatch` fix, on the other arm — and pinned this time rather than assumed.
|
|
|
|
A rule whose `branch_name` is a number makes the classifier throw on a read that plainly
|
|
succeeded. Mapping that to `bp_code=000` produced "could not read … (HTTP '000' — Gitea
|
|
unreachable…)", stating a transport cause for a 200. The decision (ask) was always safe; only
|
|
the reason lied, which is precisely the defect corrected one arm over for the deny.
|
|
|
|
Parametrised over all three shapes that reach a 200 the hook cannot use, because the first fix
|
|
covered only `UNPARSEABLE-RULES` — the arm where it was noticed. `GARBAGE` (an object, not an
|
|
array) and `EMPTY` are diverted one branch EARLIER, by the array gate, and kept `bp_code=200`,
|
|
so they reported "HTTP '200' — Gitea unreachable" about a successful read. Fixing one arm and
|
|
leaving its twin is the shape this PR is largely about.
|
|
"""
|
|
hook.set_branch_protection(shape)
|
|
reason = hook.reason()
|
|
assert "ask" in reason, "an unparseable rule list produced a decision instead of a question"
|
|
assert "could not parse" in reason, "the ask blamed the transport for a 200 the classifier simply could not read"
|
|
assert "unreachable" not in reason
|
|
|
|
|
|
# ---------------------------------------------------------------------------------------------
|
|
# Guard-scope freshness (ersatztv#787). `test_ci_dropped_step_guard.py` derives which jobs must
|
|
# carry per-step execution markers from `.gitea/required-status-contexts.json`; these arms are what
|
|
# stop that snapshot going quietly stale.
|
|
# ---------------------------------------------------------------------------------------------
|
|
|
|
|
|
@pytest.mark.parametrize("scheduled", [True, False], ids=["SCHEDULED", "IMMEDIATE"])
|
|
def test_a_DRIFTED_contexts_snapshot_ASKS_on_BOTH_merge_paths(hook, scheduled):
|
|
"""BOTH paths, and that parametrization is the point rather than thoroughness for its own sake.
|
|
|
|
The branch-protection read this repo already had lives inside the `merge_when_checks_succeed`
|
|
branch and never runs on an immediate merge — the common case. Hanging the freshness check off
|
|
it would have fired it only when an auto-merge was armed. This file already records that exact
|
|
shape once: the base re-read "first landed inside the scheduled-auto-merge branch only", and
|
|
cold review found scheduled+retarget denied while immediate+retarget auto-GRANTED.
|
|
"""
|
|
hook.set_branch_protection("DRIFTED-CONTEXTS")
|
|
decision = hook.decision(scheduled=scheduled)
|
|
assert decision is not None, "the hook passed through instead of deciding"
|
|
out = decision["hookSpecificOutput"]
|
|
assert out["permissionDecision"] == "ask", f"a stale guard scope must ask, not {out['permissionDecision']!r}"
|
|
assert "required-status-contexts.json" in out["permissionDecisionReason"], (
|
|
"the ask must name the snapshot to reconcile, not merely report unease"
|
|
)
|
|
# ASK, never DENY — folded in here rather than kept as its own test. As a separate assertion on
|
|
# `!= "deny"` it was VACUOUS: deleting the whole freshness arm leaves the hook GRANTING, which
|
|
# also satisfies it, and so would any unrelated earlier ask. It only means something alongside
|
|
# the exact-`ask` and reason assertions above, which pin WHICH arm answered.
|
|
assert out["permissionDecision"] != "deny", (
|
|
"drift must not deny: Gitea enforces the live required set server-side, so this merge is "
|
|
"not unsafe — what is stale is a guard's scope, and denying would state something false "
|
|
"about the change in front of the reader"
|
|
)
|
|
|
|
|
|
def test_the_freshness_check_does_not_PREEMPT_a_more_serious_refusal(hook):
|
|
"""ORDERING, asserted rather than described. The freshness check is last because it can only
|
|
downgrade a grant, and a reader whose merge is blocked for a more serious reason must be told
|
|
THAT reason. Here branch protection does not require review-verdict/h10 at all: the deny for
|
|
that must win over the freshness ask, even though the contexts also differ from the snapshot."""
|
|
hook.set_branch_protection("MISSING-CONTEXT")
|
|
out = hook.decision()["hookSpecificOutput"]
|
|
assert out["permissionDecision"] == "deny"
|
|
assert "review-verdict/h10' is NOT a required status check" in out["permissionDecisionReason"]
|
|
assert "required-status-contexts.json" not in out["permissionDecisionReason"]
|
|
|
|
|
|
@pytest.mark.parametrize("scheduled", [True, False], ids=["SCHEDULED", "IMMEDIATE"])
|
|
def test_a_healthy_repo_still_auto_grants_with_the_freshness_check_in_place(hook, scheduled):
|
|
"""NEGATIVE CONTROL. Every test above passes if the freshness check simply asks on everything.
|
|
|
|
Parametrized over BOTH paths deliberately: the IMMEDIATE path is where the branch-protection
|
|
fetch is NEW, so it is the one where an over-eager arm would silently cost the ersatztv#314
|
|
auto-grant — and it is the path a default `scheduled=True` control would never have covered.
|
|
"""
|
|
out = hook.decision(scheduled=scheduled)["hookSpecificOutput"]
|
|
assert out["permissionDecision"] == "allow", (
|
|
f"the freshness check broke the grant path (got {out['permissionDecision']!r}: "
|
|
f"{out['permissionDecisionReason'][:200]})"
|
|
)
|
|
|
|
|
|
def test_a_MISSING_checker_asks_rather_than_skipping_the_freshness_check(hook, tmp_path):
|
|
"""A guard whose implementation has gone missing must ASK, never quietly not run.
|
|
|
|
The `unknown must FAIL, not warn` arm: a checker that cannot be executed produces no comparison,
|
|
and no comparison is not a passing one. The hook resolves the checker from its OWN location
|
|
(`$repo_root`) rather than `$CLAUDE_PROJECT_DIR`, because an env var is not a sound input to a
|
|
security decision — so this exercises it by running a COPY of the hook from a tree that is
|
|
complete except for the checker, which is the only way to move `$repo_root`.
|
|
"""
|
|
project = tmp_path / "repo"
|
|
(project / ".claude" / "hooks").mkdir(parents=True)
|
|
(project / "scripts").mkdir()
|
|
(project / ".gitea").mkdir()
|
|
# Everything the arms BEFORE this one resolve, so the decision isolates the missing checker
|
|
# rather than tripping the changed-files or verdict-classifier arms.
|
|
for rel in ("scripts/pr-changed-files.sh", "scripts/check-review-verdict.sh"):
|
|
shutil.copy2(REPO_ROOT / rel, project / rel)
|
|
shutil.copytree(REPO_ROOT / "scripts" / "lib", project / "scripts" / "lib")
|
|
shutil.copy2(REPO_ROOT / ".gitea" / "required-status-contexts.json", project / ".gitea")
|
|
copied_hook = project / ".claude" / "hooks" / HOOK.name
|
|
shutil.copy2(HOOK, copied_hook)
|
|
assert not (project / "scripts" / "check-required-contexts.sh").exists()
|
|
|
|
# IMMEDIATE path, so the scheduled branch's own branch-protection read cannot muddy the
|
|
# no-fetch assertion below.
|
|
out = hook.decision(scheduled=False, project_dir=str(project), hook_path=copied_hook)["hookSpecificOutput"]
|
|
assert out["permissionDecision"] == "ask", (
|
|
f"a missing checker must ask, not {out['permissionDecision']!r} — no comparison ran"
|
|
)
|
|
# WORDING UNIQUE TO THIS ARM, and then the behaviour only this arm produces. Asserting the
|
|
# substring `check-required-contexts.sh` was NOT enough: delete the `[ ! -x ]` clause and the
|
|
# exec simply fails, `ctx_class` is empty, and the catch-all arm asks with a reason ending
|
|
# "Check scripts/check-required-contexts.sh." — satisfying both of the original assertions while
|
|
# proving nothing about the clause this test names. The two guards then mask each other (#685).
|
|
assert "missing or not executable" in out["permissionDecisionReason"], (
|
|
f"the ask came from a different arm: {out['permissionDecisionReason'][:200]}"
|
|
)
|
|
assert not hook.branch_protection_urls(), (
|
|
"the hook fetched branch protection before noticing its checker was missing — only the "
|
|
"early `[ ! -x ]` clause prevents that fetch, so this proves the clause ran"
|
|
)
|
|
|
|
|
|
def test_an_UNREADABLE_class_gets_its_OWN_arm_not_the_catch_all(hook):
|
|
"""`unreadable` is a class the checker's contract DECLARES. Letting it fall through to the
|
|
catch-all asks for the right reason with the wrong explanation — it tells the operator the
|
|
checker returned a word the hook does not understand and sends them to debug the script, when
|
|
what actually happened is that branch protection or the snapshot came back malformed."""
|
|
hook.set_branch_protection("NON-STRING-MEMBER")
|
|
# THE IMMEDIATE PATH, deliberately. On the scheduled path the h10 required-check arm sees the
|
|
# same malformed payload and asks first, so asserting there proves nothing about this arm — the
|
|
# first draft of this test did exactly that and passed without ever reaching the code it names.
|
|
reason = hook.decision(scheduled=False)["hookSpecificOutput"]["permissionDecisionReason"]
|
|
assert "could not consume" in reason, f"the unreadable arm did not fire; some other arm answered: {reason[:200]}"
|
|
assert "not a class this hook understands" not in reason, (
|
|
f"a declared class reached the catch-all arm: {reason[:200]}"
|
|
)
|
|
|
|
|
|
def test_the_repo_gate_is_CASE_INSENSITIVE_like_Gitea_itself(hook):
|
|
"""Gitea resolves owner/repo case-insensitively — verified live, `/repos/TIMOTHY/ErsatzTV` and
|
|
`/repos/timothy/ersatztv` both answer 200. A byte-exact gate would let a case variant skip the
|
|
freshness arm entirely while every other arm still resolved, so drift would go unreported with
|
|
no ask: the gate failing open on a spelling."""
|
|
hook.set_branch_protection("DRIFTED-CONTEXTS")
|
|
out = hook.decision(owner="TIMOTHY", repo="ErsatzTV")["hookSpecificOutput"]
|
|
assert out["permissionDecision"] == "ask", (
|
|
f"a case variant of this repo skipped the freshness arm (got {out['permissionDecision']!r})"
|
|
)
|
|
assert "required-status-contexts.json" in out["permissionDecisionReason"]
|
|
|
|
|
|
def test_the_freshness_arm_does_NOT_fire_for_a_DIFFERENT_repo(hook):
|
|
"""The arm compares a hardcoded `main` against a snapshot committed in THIS checkout, while the
|
|
merge tool is called with whatever owner/repo the user is merging. Running it for another repo
|
|
weighs that repo's live contexts against this repo's mirror: measured, `server-management`
|
|
returns `[]`, which classifies as `nomatch` and would have asked with a reason naming
|
|
`.gitea/required-status-contexts.json` — a confident statement about a repo it does not describe.
|
|
"""
|
|
# The foreign repo's protection must actually DIFFER from this repo's snapshot, or the arm would
|
|
# report `match` and stay silent whether or not it ran — which is how the first draft of this
|
|
# test passed with the repo gate disabled. Asserting the absence of a string only means something
|
|
# when the string WOULD be there without the guard.
|
|
hook.set_branch_protection("DRIFTED-CONTEXTS")
|
|
out = hook.decision(repo="server-management")["hookSpecificOutput"]
|
|
assert out["permissionDecision"] == "allow", (
|
|
"a repo this snapshot does not describe must not be judged by it; the gate should be "
|
|
f"skipped and the merge auto-granted, got {out['permissionDecision']!r}: "
|
|
f"{out['permissionDecisionReason'][:200]}"
|
|
)
|
|
assert "required-status-contexts.json" not in out["permissionDecisionReason"], (
|
|
"the guard-scope freshness arm fired for a repo its snapshot does not describe: "
|
|
f"{out['permissionDecisionReason'][:200]}"
|
|
)
|
|
|
|
|
|
def test_the_HOOK_loads_the_SHARED_classifier_behaviourally(hook, tmp_path):
|
|
"""The other half of the one-copy property, proven by swapping the file rather than reading source.
|
|
|
|
Cold review re-inlined a BYTE-IDENTICAL copy of the classifier into this hook, left a comment
|
|
naming the shared path above it, and the whole suite stayed green — a substring assertion cannot
|
|
tell a loaded file from a mentioned one, and the byte-identical inline is exactly the refactor
|
|
that drifts later because it agrees today. Replacing the shared program with a sentinel that can
|
|
only answer `undecidable` must therefore change what the hook DECIDES.
|
|
"""
|
|
project = tmp_path / "repo"
|
|
(project / ".claude" / "hooks").mkdir(parents=True)
|
|
(project / "scripts").mkdir()
|
|
(project / ".gitea").mkdir()
|
|
for rel in ("scripts/pr-changed-files.sh", "scripts/check-review-verdict.sh", "scripts/check-required-contexts.sh"):
|
|
shutil.copy2(REPO_ROOT / rel, project / rel)
|
|
(project / "scripts" / "lib").mkdir()
|
|
(project / "scripts" / "lib" / "branch-rule-classifier.jq").write_text(
|
|
'# sentinel: answers only one way, so a caller that loads THIS file changes\n{verdict: "undecidable"}\n'
|
|
)
|
|
shutil.copy2(REPO_ROOT / ".gitea" / "required-status-contexts.json", project / ".gitea")
|
|
copied_hook = project / ".claude" / "hooks" / HOOK.name
|
|
shutil.copy2(HOOK, copied_hook)
|
|
|
|
# SCHEDULED, because that is the path whose rule selection consumes the classifier.
|
|
out = hook.decision(scheduled=True, project_dir=str(project), hook_path=copied_hook)["hookSpecificOutput"]
|
|
assert out["permissionDecision"] == "ask", (
|
|
f"swapping the shared classifier did not change the hook's decision (got "
|
|
f"{out['permissionDecision']!r}) — it is not loading that file"
|
|
)
|
|
assert "decidably" in out["permissionDecisionReason"], (
|
|
"the hook decided for some other reason than the sentinel classifier's `undecidable`: "
|
|
f"{out['permissionDecisionReason'][:200]}"
|
|
)
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
("mode", "phrase"),
|
|
[
|
|
pytest.param("EMPTY-LIST", "no branch-protection rule governs 'main' at all", id="nomatch"),
|
|
pytest.param("GLOB-RULE", "a glob branch-protection rule could govern 'main'", id="undecidable"),
|
|
pytest.param("LIST-UNREADABLE", "could not read branch protection for the guard-scope", id="readfail"),
|
|
],
|
|
)
|
|
def test_each_remaining_freshness_class_gets_its_OWN_arm(hook, mode, phrase):
|
|
"""One arm of four had a proof; the other three were carried by the catch-all.
|
|
|
|
Fail-safe is not the same as proven: this file's own argument in the `unreadable` test — an arm
|
|
that asks "for the right reason with the wrong explanation" — applies verbatim to `nomatch`,
|
|
`undecidable` and `readfail`. Measured before this test existed: deleting all three arms left the
|
|
file green at 47 passed, because the catch-all answered for them and blamed the checker.
|
|
|
|
The IMMEDIATE path throughout: on the scheduled path the h10 required-check arm consumes these
|
|
same payloads and answers first, which is how an earlier test in this file passed without
|
|
reaching the code it named.
|
|
"""
|
|
hook.set_branch_protection(mode)
|
|
reason = hook.decision(scheduled=False)["hookSpecificOutput"]["permissionDecisionReason"]
|
|
assert phrase in reason, f"the {mode} payload did not reach its own arm: {reason[:220]}"
|
|
assert "not a class this hook understands" not in reason, (
|
|
f"a declared class fell through to the catch-all: {reason[:220]}"
|
|
)
|
|
|
|
|
|
def test_a_checker_USAGE_error_reaches_the_operator_WITH_its_cause(hook, tmp_path):
|
|
"""The catch-all must name what actually happened, not just that something did.
|
|
|
|
The checker exits 2 with a diagnostic on a usage error — an unreadable snapshot, a branch
|
|
mismatch, a missing shared classifier. While its stderr went to /dev/null every one of those
|
|
arrived as "returned 'nothing'", which names no cause: the same states-a-cause-that-did-not-happen
|
|
shape this arm avoids elsewhere. Here the classifier is absent, so the checker exits 2 saying so.
|
|
"""
|
|
project = tmp_path / "repo"
|
|
(project / ".claude" / "hooks").mkdir(parents=True)
|
|
(project / "scripts").mkdir()
|
|
(project / ".gitea").mkdir()
|
|
for rel in ("scripts/pr-changed-files.sh", "scripts/check-review-verdict.sh", "scripts/check-required-contexts.sh"):
|
|
shutil.copy2(REPO_ROOT / rel, project / rel)
|
|
# `scripts/lib/` IS copied, minus the one file under test: omitting the whole directory also
|
|
# removes `review-verdict-vocabulary.sh`, which `check-review-verdict.sh` sources — and that arm
|
|
# answers first, so the test would assert against a completely different failure.
|
|
shutil.copytree(REPO_ROOT / "scripts" / "lib", project / "scripts" / "lib")
|
|
(project / "scripts" / "lib" / "branch-rule-classifier.jq").unlink()
|
|
shutil.copy2(REPO_ROOT / ".gitea" / "required-status-contexts.json", project / ".gitea")
|
|
copied_hook = project / ".claude" / "hooks" / HOOK.name
|
|
shutil.copy2(HOOK, copied_hook)
|
|
|
|
out = hook.decision(scheduled=False, project_dir=str(project), hook_path=copied_hook)["hookSpecificOutput"]
|
|
assert out["permissionDecision"] == "ask"
|
|
assert "classifier not readable" in out["permissionDecisionReason"], (
|
|
"the checker's own diagnostic was discarded, so the operator is told only that it returned "
|
|
f"nothing: {out['permissionDecisionReason'][:220]}"
|
|
)
|