From 72165b5db5ff3b03aaf99b473f8e201cbe5fb402 Mon Sep 17 00:00:00 2001 From: claude Date: Sun, 26 Jul 2026 17:59:21 +0000 Subject: [PATCH] fix(git-gate): reject AGit review refs in pre-receive Gitea AGit accepts pushes to refs/for/*, refs/draft/*, and refs/for-review/* and opens pull requests backed by server-managed refs/pull//head refs rather than ordinary refs/heads/* branches. That breaks the git-gate branch workflow: follow-up commits can't be pushed back through the branch, and Gitea rejects later direct updates to the generated review ref, so recovery means recreating the PR. Add a Phase 0 guard to the shared pre-receive hook that rejects creation or update of those AGit review refs before any gitleaks scan or upstream forward, with a message pointing callers at the branch-backed PR workflow. Deletions (new == zero) stay allowed so legacy AGit refs can still be cleaned up; normal branches and tags are untouched. Closes #506 Co-Authored-By: Claude Opus 4.8 --- bot_bottle/gateway/git_gate/render.py | 21 ++++++++ tests/unit/test_git_gate.py | 69 +++++++++++++++++++++++++++ 2 files changed, 90 insertions(+) diff --git a/bot_bottle/gateway/git_gate/render.py b/bot_bottle/gateway/git_gate/render.py index 241ed9d8..515db6fc 100644 --- a/bot_bottle/gateway/git_gate/render.py +++ b/bot_bottle/gateway/git_gate/render.py @@ -252,6 +252,27 @@ cat > "$refs_file" zero=0000000000000000000000000000000000000000 +# Phase 0: reject Gitea AGit review refs before scanning or forwarding. +# A push to refs/for/*, refs/draft/*, or refs/for-review/* asks Gitea to +# open a pull request backed by a server-managed refs/pull//head rather +# than an ordinary refs/heads/* branch. That breaks the git-gate workflow: +# follow-up commits can't be pushed back through the branch, and Gitea +# rejects later direct updates to the generated review ref. Fail the whole +# push here (before any gitleaks scan or upstream forward) so the caller +# pushes a real branch and opens the PR against it instead. Deletions +# (new == zero) stay allowed so stale AGit refs can still be cleaned up. +while IFS=' ' read -r old new ref; do + [ -z "$ref" ] && continue + [ "$new" = "$zero" ] && continue + case "$ref" in + refs/for/*|refs/draft/*|refs/for-review/*) + echo "git-gate: refusing AGit review ref $ref" >&2 + echo "git-gate: push to refs/heads/ and open a branch-backed pull request instead" >&2 + exit 1 + ;; + esac +done < "$refs_file" + supervise_gitleaks_allow() { log_opts=$1 ref=$2 diff --git a/tests/unit/test_git_gate.py b/tests/unit/test_git_gate.py index bf7a891d..34e0ce55 100644 --- a/tests/unit/test_git_gate.py +++ b/tests/unit/test_git_gate.py @@ -206,6 +206,75 @@ class TestHookRender(unittest.TestCase): self.assertIn('set -- "$@" --push-option="$opt"', hook) self.assertIn('git push "$@" origin "$refspec"', hook) + def test_agit_review_refs_rejected_before_scan(self): + # Creating/updating refs/for/*, refs/draft/*, or refs/for-review/* + # opens a Gitea AGit pull request backed by a server-managed review + # ref instead of a normal branch, which the git-gate branch workflow + # can't push follow-ups to. The guard rejects those refs, and it runs + # in Phase 0 — before the gitleaks scan and the upstream forward. + hook = git_gate_render_hook() + self.assertIn( + "refs/for/*|refs/draft/*|refs/for-review/*", hook, + ) + self.assertIn("refusing AGit review ref", hook) + self.assertIn("branch-backed pull request", hook) + guard = hook.index("refusing AGit review ref") + self.assertLess( + guard, hook.index("gitleaks scanning"), + "AGit guard must run before the gitleaks scan", + ) + self.assertLess( + guard, hook.index("forwarding $ref to origin"), + "AGit guard must run before the upstream forward", + ) + + def test_agit_review_ref_deletion_still_allowed(self): + # Cleanup of a legacy AGit ref (new == zero) must not be blocked, so + # the reject is guarded by the same delete short-circuit the scan and + # forward phases use. + hook = git_gate_render_hook() + guard_block = hook[ + hook.index("Phase 0"):hook.index("supervise_gitleaks_allow()") + ] + self.assertIn('[ "$new" = "$zero" ] && continue', guard_block) + self.assertIn("refs/for/*", guard_block) + + def _run_hook_stdin(self, stdin: str): + # Execute the rendered hook far enough to exercise Phase 0. The guard + # touches only mktemp + read, so it rejects (or falls through) without + # a bare repo, gitleaks, or ssh — anything past Phase 0 fails for + # unrelated reasons, which is fine for the reject cases asserted here. + import subprocess + fd, path = tempfile.mkstemp(suffix=".sh") + try: + with os.fdopen(fd, "w") as f: + f.write(git_gate_render_hook()) + return subprocess.run( + ["sh", path], input=stdin, capture_output=True, text=True, + ) + finally: + os.unlink(path) + + def test_agit_review_ref_create_is_rejected(self): + one = "1" * 40 + for ref in ("refs/for/main", "refs/draft/main", "refs/for-review/main"): + with self.subTest(ref=ref): + result = self._run_hook_stdin(f"{'0' * 40} {one} {ref}\n") + self.assertEqual(1, result.returncode) + self.assertIn("refusing AGit review ref", result.stderr) + self.assertIn("branch-backed pull request", result.stderr) + # Rejected in Phase 0, before the gitleaks scan runs. + self.assertNotIn("gitleaks scanning", result.stderr) + + def test_ordinary_branch_passes_agit_guard(self): + # A normal refs/heads push must fall through Phase 0 and reach the + # gitleaks scan (which then fails for lack of a real repo — proving + # only that the guard did not short-circuit it). + one = "1" * 40 + result = self._run_hook_stdin(f"{'0' * 40} {one} refs/heads/main\n") + self.assertNotIn("refusing AGit review ref", result.stderr) + self.assertIn("gitleaks scanning", result.stderr) + def test_inline_gitleaks_allow_routes_to_supervisor(self): hook = git_gate_render_hook() # First gitleaks runs normally; only if that passes does the