Files
Gitea-Tools/tests/test_review_pr.py
T
sysadminandClaude Fable 5 30cff19a41 fix(reviewer-workflow): address review — in-process mutation authority, no /tmp lock (#199)
Addresses the sysadmin REQUEST_CHANGES on PR #203 (reviewed head
10d2644790):

1. Lock redesigned; /tmp file removed entirely. The mutation authority is
   now an in-process record (_MUTATION_AUTHORITY) plus an environment
   session lock (GITEA_SESSION_PROFILE_LOCK) exported at server launch:
   - in-process record cannot be spoofed by other local processes, cannot
     go stale across sessions, and cannot race concurrent agents;
   - the env lock is inherited by child CLI processes, so review_pr.py can
     refuse an ad-hoc GITEA_MCP_PROFILE role escalation without any shared
     file; a missing env lock (direct operator CLI use) stays allowed;
   - silent except-pass writes are gone; an unresolvable profile fails
     closed.
2. Standard reviewer workflow unbroken: verify_mutation_authority seeds
   itself from the live config-resolved context at the first mutation gate
   (approved preflight path whoami -> eligibility -> review/merge), and now
   runs as the final gate after eligibility, reusing the identity that
   eligibility proved (no extra /user call).
3. Trailing whitespace removed from review_pr.py (git diff --check clean).
4. Module-global verify_mutation_authority no-op bypass removed from
   tests/test_mcp_server.py; replaced with a tests/conftest.py autouse
   fixture that only resets per-process state (_MUTATION_AUTHORITY,
   _IDENTITY_CACHE, session lock env) between tests — the gate itself
   stays live in every test.
5. Tests rewritten for the new design: seeding on first verify, unresolved
   profile fails closed, remote/profile/identity mismatches fail closed,
   session-lock env mismatch rejected, foreign-pid authority reseeded,
   unauthorized author->reviewer pivot blocked, authorized pivot allowed;
   CLI: mismatch blocked, match allowed, no-lock allowed.
6. Rebased onto current master (c6fd0fd).

Closes #199
Refs #194

Co-Authored-By: Claude Fable 5 <[email protected]>
2026-07-05 16:51:34 -04:00

166 lines
6.1 KiB
Python

"""Tests for review_pr.py.
Mocks api_request and credentials.
"""
import io
import os
import sys
import unittest
from unittest.mock import patch
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
import review_pr # noqa: E402
FAKE_CREDS = "Basic dGVzdHVzZXI6dGVzdHBhc3M="
FAKE_PR_DATA = {
"number": 81,
"state": "open",
"head": {
"ref": "feature-branch",
"sha": "abcdef1234567890"
},
"base": {
"ref": "main"
},
"html_url": "https://gitea.example.com/pulls/81"
}
class TestArgParsing(unittest.TestCase):
def setUp(self):
self.exists_patcher = patch("os.path.exists", return_value=False)
self.exists_patcher.start()
self.addCleanup(self.exists_patcher.stop)
@patch("review_pr.get_auth_header", return_value=FAKE_CREDS)
def test_missing_pr_number_exits(self, _auth):
with self.assertRaises(SystemExit):
review_pr.main([])
class TestAPIPayload(unittest.TestCase):
def setUp(self):
self.exists_patcher = patch("os.path.exists", return_value=False)
self.exists_patcher.start()
self.addCleanup(self.exists_patcher.stop)
@patch("review_pr.api_request")
@patch("review_pr.get_auth_header", return_value=FAKE_CREDS)
def test_payload_fields_and_workflow(self, _auth, mock_api):
# Setup mock api_request to return PR details, then review response
mock_api.side_effect = [FAKE_PR_DATA, {}]
rc = review_pr.main([
"--pr-number", "81",
"--event", "APPROVE",
"--body", "Approved and ready to merge",
])
self.assertEqual(rc, 0)
self.assertEqual(mock_api.call_count, 2)
# Verify first call: GET PR
first_call_args = mock_api.call_args_list[0]
self.assertEqual(first_call_args[0][0], "GET")
self.assertEqual(first_call_args[0][1], "https://gitea.dadeschools.net/api/v1/repos/Contractor/Timesheet/pulls/81")
# Verify second call: POST review
second_call_args = mock_api.call_args_list[1]
self.assertEqual(second_call_args[0][0], "POST")
self.assertEqual(second_call_args[0][1], "https://gitea.dadeschools.net/api/v1/repos/Contractor/Timesheet/pulls/81/reviews")
payload = second_call_args[0][3]
self.assertEqual(payload["event"], "APPROVE")
self.assertEqual(payload["body"], "Approved and ready to merge")
self.assertEqual(payload["commit_id"], "abcdef1234567890")
@patch("review_pr.api_request")
@patch("review_pr.get_auth_header", return_value=FAKE_CREDS)
def test_merge_flag_fails_closed_without_api_call(self, _auth, mock_api):
# --merge is an ungated bypass and is disabled (#16). It must fail
# closed BEFORE any API call — no review, no merge.
rc = review_pr.main([
"--pr-number", "81",
"--event", "APPROVE",
"--body", "Approved",
"--merge",
"--merge-method", "squash",
])
self.assertEqual(rc, 2)
self.assertEqual(mock_api.call_count, 0)
def test_merge_flag_message_points_to_gated_workflow(self):
from _pytest.monkeypatch import MonkeyPatch
import io
with patch("review_pr.get_auth_header", return_value=FAKE_CREDS), \
patch("review_pr.api_request") as mock_api:
buf = io.StringIO()
monkeypatch = MonkeyPatch()
monkeypatch.setattr(sys, "stderr", buf)
try:
rc = review_pr.main([
"--pr-number", "81", "--event", "APPROVE", "--merge",
])
finally:
monkeypatch.undo()
self.assertEqual(rc, 2)
self.assertEqual(mock_api.call_count, 0)
msg = buf.getvalue().lower()
self.assertIn("disabled", msg)
self.assertIn("gitea_merge_pr", msg)
class TestMutationAuthorityLock(unittest.TestCase):
"""#199 (refs #194): the CLI refuses to run under a profile that differs
from the session profile lock exported by the launching MCP session."""
@patch("review_pr.get_profile")
def test_cli_blocked_on_session_lock_mismatch(self, mock_get_profile):
# An author-bound session exported the lock; the CLI resolves a
# reviewer profile (GITEA_MCP_PROFILE side-channel override) — reject
# before any API call.
mock_get_profile.return_value = {"profile_name": "prgs-reviewer"}
import io
buf = io.StringIO()
with patch.dict(os.environ,
{"GITEA_SESSION_PROFILE_LOCK": "prgs-author"}), \
patch.object(sys, "stderr", buf):
rc = review_pr.main([
"--pr-number", "81", "--event", "APPROVE",
])
self.assertEqual(rc, 3)
msg = buf.getvalue().lower()
self.assertIn("cli override rejected", msg)
@patch("review_pr.get_profile")
@patch("review_pr.api_request")
@patch("review_pr.get_auth_header", return_value=FAKE_CREDS)
def test_cli_allowed_on_profile_match(self, _auth, mock_api, mock_get_profile):
mock_get_profile.return_value = {"profile_name": "prgs-reviewer"}
mock_api.side_effect = [FAKE_PR_DATA, {}]
with patch.dict(os.environ,
{"GITEA_SESSION_PROFILE_LOCK": "prgs-reviewer"}):
rc = review_pr.main([
"--pr-number", "81", "--event", "APPROVE",
])
self.assertEqual(rc, 0)
@patch("review_pr.api_request")
@patch("review_pr.get_auth_header", return_value=FAKE_CREDS)
def test_cli_allowed_without_session_lock(self, _auth, mock_api):
# No lock in the environment = direct operator CLI use; the wall
# does not apply and the normal flow proceeds.
mock_api.side_effect = [FAKE_PR_DATA, {}]
env = {k: v for k, v in os.environ.items()
if k != "GITEA_SESSION_PROFILE_LOCK"}
with patch.dict(os.environ, env, clear=True):
rc = review_pr.main([
"--pr-number", "81", "--event", "APPROVE",
])
self.assertEqual(rc, 0)
if __name__ == "__main__":
unittest.main()