fix(743): make review-verdict/h10 unskippable — disable direct push to main + admin force-merge; fix(719) tag-only push #749

Merged

timothy merged 5 commits from fix/743-push-protection into main 2026-08-06 00:58:17 +02:00

Conversation 1 Commits 5 Files Changed 10

+401 -19

timothy commented 2026-08-06 00:11:23 +02:00

Owner

Copy Link

Copy Source

Closes the last no-forgery bypass of the review-verdict/h10 merge gate, and unblocks the release cut that H11 was breaking.

fixes #743
fixes #719

#743 — the gate was on one of two paths to main

Gitea evaluates status_check_contexts when it merges a PR. A direct git push origin HEAD:main never consults them, so the entire h10 apparatus — per-sha binding (#622), base-resolved definition (#672), exemption provenance (#698), retarget abstention (#706) — defended a path you could simply not take. That was strictly cheaper than every route enumerated in #697.

main now carries two fields, and the second one is the part that is easy to miss:

Field	Closes
enable_push: false	direct push, contents API, web-editor/upload paths
block_admin_merge_override: true	admin force_merge straight past a missing or red h10

Disabling push alone was not enough, and the first review round caught that. The push argument was "a whitelist naming timothy closes nothing, because timothy is the identity every credential already holds" — and the merge path had the identical shape, left unchecked. An admin-shaped control that exempts the only admin exempts everybody.

Measured, not assumed

Probed against throwaway probe-743-* rules rather than against main:

enable_push: false                    -> push by timothy (site admin): REFUSED (pre-receive)
enable_push_whitelist + ["timothy"]   -> identical push: SUCCEEDED
contents API PUT to protected branch  -> 403 "user cannot commit to repo"

That second line is why this is a disable, not a whitelist — #743 offered the two as interchangeable and they are not. Then on main itself: direct push REFUSED, tag-only push SUCCEEDED from the same worktree. All probe artifacts deleted; origin/main unchanged at 08e95f9ec throughout.

The admin force-merge bypass is labelled source-attested, not probed — probing it means merging an unreviewed PR.

What it does not close

An admin credential can PATCH either field off. Recorded as an accepted residual, not implied to be covered. What it does close is every write-only credential route: the injected GITEA_TOKEN, RENOVATE_TOKEN, non-admin collaborator PATs.

#719 — H11 blocked every release cut

The release cut tags from a branch behind origin/main, so H11 fired and printed rebase advice that did not even apply (no branch was being pushed). Now exempt when every pushed ref is under refs/tags/.

Two details are load-bearing and both are pinned by tests:

• .husky/pre-push had to forward the ref lines it already consumed — without that the check gets EOF and the exemption is dead code. The unit tests drive the hook directly and would still pass, so this wiring is deliberately called out in the record.
• The exemption requires at least one parsed ref line. "All refs are tags" is vacuously true over zero lines, which would silently disable H11 entirely.

Verification

• 441/441 script tests; decisions-validate OK; PyYAML parses all 193 records.
• Integration-verified end-to-end against a genuinely-behind branch: tag-only push proceeds past H11, branch push from the same branch is blocked.
• Mutation-tested. Two review-round fixes had SURVIVING mutants (correct but unpinned) and now have tests that kill them — including a pty test for the TTY-hang guard, with a timeout so a regression fails cleanly instead of hanging CI.

Docs

ci-cd.md, ci.actions-credential-scoping, release.verdict-status-check and release.format-as-you-touch-rebase updated; new record release.main-direct-push-disabled. The upgrade to Gitea 1.27.1 mid-session also invalidated a directive (permissions: "while below 1.26.0"), corrected without overclaiming that the constraint now binds — that is unprobed, and tracked in #747/#748.

Deferred with issues filed: #746 (persist-credentials, after the || true fetch masking is removed), #747 (re-verify 1.25.4-pinned claims).

Reviewed adversarially in two cold rounds; round 1 returned BLOCKED (admin force-merge), round 2 returned BLOCKED (a third stale present-tense claim). Both fixed here.

Closes the last no-forgery bypass of the `review-verdict/h10` merge gate, and unblocks the release cut that H11 was breaking. fixes #743 fixes #719 ## #743 — the gate was on one of two paths to `main` Gitea evaluates `status_check_contexts` when it **merges a PR**. A direct `git push origin HEAD:main` never consults them, so the entire h10 apparatus — per-sha binding (#622), base-resolved definition (#672), exemption provenance (#698), retarget abstention (#706) — defended a path you could simply not take. That was strictly cheaper than every route enumerated in #697. `main` now carries **two** fields, and the second one is the part that is easy to miss: | Field | Closes | |---|---| | `enable_push: false` | direct push, contents API, web-editor/upload paths | | `block_admin_merge_override: true` | admin `force_merge` straight past a missing or red `h10` | **Disabling push alone was not enough**, and the first review round caught that. The push argument was "a whitelist naming `timothy` closes nothing, because `timothy` is the identity every credential already holds" — and the merge path had the identical shape, left unchecked. An admin-shaped control that exempts the only admin exempts everybody. ### Measured, not assumed Probed against throwaway `probe-743-*` rules rather than against `main`: ``` enable_push: false -> push by timothy (site admin): REFUSED (pre-receive) enable_push_whitelist + ["timothy"] -> identical push: SUCCEEDED contents API PUT to protected branch -> 403 "user cannot commit to repo" ``` That second line is why this is a *disable*, not a whitelist — #743 offered the two as interchangeable and they are not. Then on `main` itself: direct push REFUSED, tag-only push SUCCEEDED from the same worktree. All probe artifacts deleted; `origin/main` unchanged at `08e95f9ec` throughout. The admin force-merge bypass is labelled **source-attested, not probed** — probing it means merging an unreviewed PR. ### What it does not close An admin credential can `PATCH` either field off. Recorded as an accepted residual, not implied to be covered. What it does close is every write-only credential route: the injected `GITEA_TOKEN`, `RENOVATE_TOKEN`, non-admin collaborator PATs. ## #719 — H11 blocked every release cut The release cut tags from a branch behind `origin/main`, so H11 fired and printed rebase advice that did not even apply (no branch was being pushed). Now exempt when **every** pushed ref is under `refs/tags/`. Two details are load-bearing and both are pinned by tests: - `.husky/pre-push` had to **forward** the ref lines it already consumed — without that the check gets EOF and the exemption is dead code. The unit tests drive the hook directly and would still pass, so this wiring is deliberately called out in the record. - The exemption requires **at least one** parsed ref line. "All refs are tags" is vacuously true over zero lines, which would silently disable H11 entirely. ## Verification - 441/441 script tests; `decisions-validate` OK; PyYAML parses all 193 records. - Integration-verified end-to-end against a genuinely-behind branch: tag-only push proceeds past H11, branch push from the *same* branch is blocked. - Mutation-tested. Two review-round fixes had SURVIVING mutants (correct but unpinned) and now have tests that kill them — including a pty test for the TTY-hang guard, with a timeout so a regression fails cleanly instead of hanging CI. ## Docs `ci-cd.md`, `ci.actions-credential-scoping`, `release.verdict-status-check` and `release.format-as-you-touch-rebase` updated; new record `release.main-direct-push-disabled`. The upgrade to Gitea 1.27.1 mid-session also invalidated a *directive* (`permissions:` "while below 1.26.0"), corrected without overclaiming that the constraint now binds — that is unprobed, and tracked in #747/#748. Deferred with issues filed: **#746** (persist-credentials, after the `|| true` fetch masking is removed), **#747** (re-verify 1.25.4-pinned claims). > Reviewed adversarially in two cold rounds; round 1 returned BLOCKED (admin force-merge), round 2 returned BLOCKED (a third stale present-tense claim). Both fixed here.

timothy added 4 commits 2026-08-06 00:11:24 +02:00

fix(743): disable direct pushes to main so review-verdict/h10 is on the only path to main ... 691a7acc77

Gitea evaluates `status_check_contexts` when it MERGES a PR. A direct
`git push origin HEAD:main` never consults them, so the whole h10 gate was
skippable with no forgery — strictly cheaper than every route enumerated in
#697. `main` now carries `enable_push: false`.

Measured on this instance (Gitea 1.27.1) against a throwaway `probe-743-*`
rule rather than against `main`:

  enable_push: false                      -> push by timothy (site admin)
                                             REFUSED, pre-receive hook declined
  enable_push_whitelist + ["timothy"]     -> identical push SUCCEEDED

That second line is why this is a DISABLE and not a whitelist: #743 offered the
two as interchangeable, but the only write accounts here are `timothy` (site
admin) and `renovate`, and every credential in the threat model — agent
sessions, PATs, the injected GITEA_TOKEN — acts as `timothy`. A whitelist
naming `timothy` would have ticked the box and closed nothing.

Then demonstrated on `main` itself, per the issue's Done-when: a direct push
was refused, and a tag-only push from the same worktree succeeded (tags are
governed by `tag_protections`, which is empty). The release cut is unaffected.

What this closes: the write-only credential routes — the injected GITEA_TOKEN,
RENOVATE_TOKEN, any non-admin collaborator PAT. What it does NOT close: an
admin credential can PATCH the protection off, push, and restore it. Recorded
as an accepted residual rather than implied to be covered.

Also corrects two claims the probe contradicted, and one that the mid-session
Gitea upgrade (1.25.4 -> 1.27.1) invalidated:

- ci-cd.md and ci.actions-credential-scoping both said "a push whitelist would
  close more of this class than the 1.26 upgrade". The whitelist form closes
  nothing here; corrected in place.
- ci.actions-credential-scoping's rule said "do NOT add a `permissions:` key
  while this instance is below Gitea 1.26.0". That precondition no longer
  holds at 1.27.1, so the directive now misleads. Corrected — while noting the
  consequence is still UNVERIFIED: `/api/v1/settings/actions` 404s at 1.27.1,
  so whether `permissions:` binds here was not probed. The upgrade alone is
  not evidence the constraint works.
- That record's 1.25.4 measurements are now dated, not current. Flagged as
  such rather than silently re-pinned to a version they were never taken on.

#743's fourth box (docker-build.yml `persist-credentials: false`) is decided
in the record and deliberately not done here: two of its checkout steps run
`git fetch ... || true` feeding the changed-file skip logic, so a credential
regression would be silent rather than loud. Drop the `|| true` masking first.

fixes #743

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

fix(719): exempt tag-only pushes from the H11 branch-freshness check ... b91707b707

H11 (.claude/hooks/prepush-rebase-check.sh) refuses to push a branch that is
behind origin/main. It fired on tag-only pushes too, breaking every release
cut: docs/ci-cd.md's "Cutting a release" flow lands a release-notes commit
via PR and then tags that merge commit, so the local branch is always one
commit behind origin/main at tag time. A tag push cannot revert anyone's
merged work, which is the failure H11 exists to prevent, so skip the
freshness check when every ref being pushed is under refs/tags/.

.husky/pre-push previously consumed pre-push's stdin ref lines and forwarded
them only to prepush-donewhen.sh; prepush-rebase-check.sh got none. Forward
the captured $_prepush_refs to it too, or the new logic is dead.

Guard against the vacuous-truth case explicitly required by #719: "all
pushed refs are tags" is trivially true over zero ref lines (manual run,
forgotten forwarding), which would silently disable H11 for every push.
Require at least one parsed ref line before granting the exemption.

Adds scripts/tests/test_prepush_rebase_check_tag_exemption.py using real
local git repos (bare origin + a work tree pushed one commit behind it) to
exercise git fetch/merge-base/rev-list against a genuinely-moved origin:
tag-only allowed, branch-only still blocked, mixed branch+tag still
blocked, and zero ref lines still blocked (the vacuous-truth guard).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

fix(743): close the admin force-merge bypass; the push half alone was not enough ... 6d80343320

Independent review found the record repeated on the merge path exactly the
mistake it had just diagnosed on the push path.

The push argument was: a whitelist naming `timothy` closes nothing, because
`timothy` is the identity every credential already holds. The merge path had
the identical shape and went unchecked — `block_admin_merge_override` defaults
to `false`, so `CanBypassBranchProtection` returns true for a repo admin and
`POST /pulls/{n}/merge` with `force_merge: true` merges straight past a missing
or red `review-verdict/h10`. One API call, no forgery, no PATCH — cheaper than
the push route this change had just removed.

So `enable_push: false` alone did NOT make the gate load-bearing, which is
what the record's headline sentence claimed. `main` now carries both fields;
they are one control and neither is citable alone.

An admin-shaped control that exempts the only admin exempts everybody.

Other review findings addressed:

- H11's owning record (`release.format-as-you-touch-rebase`) now documents the
  #719 tag-only carve-out. It is a narrowing of an existing convention, so it
  amends that record rather than adding a new one — including the two details
  that are easy to regress (the .husky/pre-push forwarding, without which the
  exemption is dead code the unit tests still pass over; and the at-least-one-
  ref guard against vacuous exemption).
- The record now states which write surfaces were enumerated and how each was
  established — contents-API refusal is MEASURED here (403 `user cannot commit
  to repo`), apply-patch/revert/cherry-pick are source-attested only. The
  admin force-merge bypass is likewise marked source-attested, not probed:
  probing it means merging an unreviewed PR.
- prepush-rebase-check.sh: process a final ref line with no trailing newline
  (previously dropped, which silently reinstated the #719 block), and skip the
  stdin read on a TTY so an interactive run does not hang.
- Corrected a citation the review caught: docs/ci-cd.md documents the tag step,
  not a release-notes-PR flow. Cite #719 for the observed flow instead.

Also fixed a frontmatter break this round introduced: a `: ` inside the
unquoted `rule:` scalar. PyYAML rejected it while the dependency-free reader
accepted it, so only `scripts/tests` caught it.

Verification: 438/438 script tests pass; decisions-validate OK; PyYAML parses
all three touched records.

refs #743 #719

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

fix(743): re-tense the third stale site, and pin the two surviving mutants ... 6af65ba5c5

Round 2 of review. One blocking finding, and it is the same defect class as
round 1's: a present-tense claim that this PR falsified.

`release.verdict-status-check` — the record ABOUT the h10 status check — still
said "direct pushes to `main` are server-side permitted, so the gate can be
skipped without forging anything". A reader resolving that key from the catalog
would conclude the control does not exist. Round 1 corrected `ci-cd.md` and
`ci.actions-credential-scoping` and I stopped at the two sites I had edited,
instead of sweeping the corpus by SUBJECT. Swept properly this time
(`server-side permitted`, `bypassable`, `without forging`, `push whitelist`,
`enable_push`): this was the only remaining stale site.

Test gaps the reviewer found by mutation testing, now closed. Both mutants
SURVIVED the suite as shipped — the round-1 fixes were correct but unpinned:

- dropping `|| [ -n "${_h11_local_ref:-}" ]` → an unterminated final line is
  dropped. Two directions, and the dangerous one is not the obvious one: a
  dropped *branch* line leaves only tag refs and grants the exemption to a push
  containing a branch. Both pinned.
- dropping `[ -t 0 ] ||` → the hook hangs forever on an interactive run. Pinned
  with a real pty and an explicit timeout, so a regression fails cleanly rather
  than hanging a CI job. Verified the mutant is killed by exactly that test
  (and that it dies via the timeout, 32s).

Also from review, non-blocking:

- `ci-cd.md:951` cited `enable_push: false` alone as what closed #743 — the
  precise thing the new record says never to do, since the force-merge route
  also skipped the gate with no forgery. Now cites both fields.
- `ci-cd.md` flattened measured and source-attested into one 403: only the
  contents API was probed; the web editor/upload/apply-patch paths share the
  predicate but were not. Separated.
- `format-as-you-touch-rebase` still said "the documented sequence" and
  "always" for the release-cut behind-ness. `docs/ci-cd.md` documents the tag
  step, not the release-notes-PR flow, and the frequency is attested by one
  observed cut. Attributed to #719 instead.
- Documented the operator recovery path. `block_admin_merge_override: true`
  removes the `force_merge` escape that used to unstick a wrongly-red required
  context — that escape WAS the bypass, so it is gone by design, and the
  recovery (fix the status; last resort PATCH the field, merge, set it back)
  needed to be written down rather than left implicit in a residual.

Verification: 441/441 script tests; decisions-validate OK; PyYAML parses all
193 records; both mutants confirmed killed and the hook restored byte-identical.

fixes #719

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

timothy added 1 commit 2026-08-06 00:35:45 +02:00

docs(743): label the second attested claim, close the survivor list, state the rule in CLAUDE.md ... f822e4737c

Round 3 returned MERGEABLE with three LOW documentation findings. Batched
before posting the verdict, since a new sha voids both the CI run and the
verdict.

- `ci-cd.md` labelled the unprobed half of the `enable_push` bullet but stated
  the `block_admin_merge_override` counterfactual flatly one bullet below —
  the same measured-vs-attested flattening round 2 fixed, one site over. Now
  labelled, with why it was not probed (verifying it means merging an
  unreviewed PR).
- `release.verdict-status-check` said "what survives is the forgery list
  above". That record's job is enumerating survivors, so an unqualified "what
  survives is X" reads as exhaustive — and it omitted the admin residual, which
  is a SKIP route rather than a forgery one. Added.
- `CLAUDE.md` never learned the rule. It is the always-read surface, and it
  still framed a direct `git push origin main` as a live path while describing
  a docs-only *push* exemption for a push the server now refuses. My corpus
  sweep covered `docs/` and missed the file that carries the docs-update rule.

Note on what remains unverified rather than closed: neither direction of
`block_admin_merge_override` was measured, and whether Gitea treats an ABSENT
required context as blocking (versus satisfied) is asserted by our docs but
not proven — the combined status on this PR reads `success` with
`review-verdict/h10` absent. Both belong to #747's re-verification sweep.

Verification: 441/441 script tests; decisions-validate OK.

refs #743

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

timothy commented 2026-08-06 00:57:48 +02:00

Author

Owner

Copy Link

Copy Source

Review-verdict: MERGEABLE @ f822e47

Three cold adversarial rounds: R1 BLOCKED (admin force_merge bypass past h10), R2 BLOCKED (third stale present-tense claim in release.verdict-status-check), R3 MERGEABLE with 3 LOW doc findings, all fixed in f822e4737 (docs-only delta, no code/test change). All four hook mutants confirmed dead. Unverified and tracked in #747: block_admin_merge_override semantics, and whether Gitea treats an absent required context as blocking.

Review-verdict: MERGEABLE @ f822e47 Three cold adversarial rounds: R1 BLOCKED (admin force_merge bypass past h10), R2 BLOCKED (third stale present-tense claim in release.verdict-status-check), R3 MERGEABLE with 3 LOW doc findings, all fixed in f822e4737 (docs-only delta, no code/test change). All four hook mutants confirmed dead. Unverified and tracked in #747: block_admin_merge_override semantics, and whether Gitea treats an absent required context as blocking.

timothy merged commit 3aed43c6de into main 2026-08-06 00:58:17 +02:00

timothy deleted branch fix/743-push-protection 2026-08-06 00:58:18 +02:00

timothy referenced this pull request 2026-08-06 00:59:27 +02:00

`review-verdict/h10` is bypassable without forging it: direct pushes to `main` are server-side permitted #743

timothy referenced this pull request 2026-08-06 00:59:27 +02:00

H11 pre-push guard blocks a tag-only push, on every release cut #719

timothy referenced this pull request 2026-08-06 14:36:21 +02:00

review-verdict.yml's classify/post step never executes — job goes green, review-verdict/h10 is never posted automatically #751

timothy referenced this pull request 2026-08-06 14:37:07 +02:00

Re-verify the Gitea 1.25.4-pinned CI claims after the 1.27.1 upgrade #747

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

ad-hoc

One-off / ad-hoc work not tracked by a dedicated issue

api

REST API / HTTP endpoints

bug

Something isn't working

ci-cd

Build, test, deploy pipeline

content

Channel content / schedules / playlists

dependencies

Dependency updates (Renovate)

enhancement

New feature or improvement

frontend

ChicoryTV React SPA frontend

in-progress

Claimed by an active session — do not pick up

jellyfin

Jellyfin tuner / IPTV integration

parked

Excluded from automatic queue pickup; work only when explicitly selected

priority: high

priority: low

priority: medium

review

Adversarial review finding

security

Security / vulnerability fix

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate timothy (Timothy)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: timothy/ersatztv#749