fix(git-gate): reject AGit review refs in pre-receive
tracker-policy-pr / check-pr (pull_request) Successful in 12s
test / unit (pull_request) Successful in 54s
test / integration-docker (pull_request) Successful in 1m9s
test / coverage (pull_request) Successful in 17s
prd-number-check / require-numbered-prds (pull_request) Successful in 8s
test / unit (push) Successful in 54s
Update Quality Badges / update-badges (push) Successful in 54s
lint / lint (push) Successful in 1m4s
test / integration-docker (push) Successful in 1m9s
test / coverage (push) Successful in 38s
tracker-policy-pr / check-pr (pull_request) Successful in 12s
test / unit (pull_request) Successful in 54s
test / integration-docker (pull_request) Successful in 1m9s
test / coverage (pull_request) Successful in 17s
prd-number-check / require-numbered-prds (pull_request) Successful in 8s
test / unit (push) Successful in 54s
Update Quality Badges / update-badges (push) Successful in 54s
lint / lint (push) Successful in 1m4s
test / integration-docker (push) Successful in 1m9s
test / coverage (push) Successful in 38s
Gitea AGit accepts pushes to refs/for/*, refs/draft/*, and refs/for-review/* and opens pull requests backed by server-managed refs/pull/<n>/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 <noreply@anthropic.com>
This commit was merged in pull request #509.
This commit is contained in:
@@ -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/<n>/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/<branch> and open a branch-backed pull request instead" >&2
|
||||
exit 1
|
||||
;;
|
||||
esac
|
||||
done < "$refs_file"
|
||||
|
||||
supervise_gitleaks_allow() {
|
||||
log_opts=$1
|
||||
ref=$2
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user