Merge branch 'prgs/master' into feat/issue-211-single-terminal-review-decision
This commit is contained in:
@@ -0,0 +1,28 @@
|
||||
"""Shared pytest fixtures for the Gitea-Tools test suite."""
|
||||
import sys
|
||||
|
||||
import pytest
|
||||
|
||||
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _reset_mutation_authority(monkeypatch):
|
||||
"""Isolate the in-process mutation authority between tests (#199).
|
||||
|
||||
The mutation-authority gate stays LIVE in every test — this fixture only
|
||||
clears the per-process record and the session profile lock so one test's
|
||||
seeded authority (or an intentionally mismatched one) cannot leak into
|
||||
the next test. It must never replace verify_mutation_authority with a
|
||||
no-op: individual tests that need a specific authority state set it up
|
||||
explicitly.
|
||||
"""
|
||||
monkeypatch.delenv("GITEA_SESSION_PROFILE_LOCK", raising=False)
|
||||
try:
|
||||
import mcp_server
|
||||
except Exception:
|
||||
yield
|
||||
return
|
||||
monkeypatch.setattr(mcp_server, "_MUTATION_AUTHORITY", None)
|
||||
monkeypatch.setattr(mcp_server, "_IDENTITY_CACHE", {})
|
||||
yield
|
||||
@@ -0,0 +1,154 @@
|
||||
"""Tests for the gated PR close path (Issue #216).
|
||||
|
||||
``gitea_edit_pr(state='closed')`` requires the distinct ``gitea.pr.close``
|
||||
capability: without it the close fails closed (no auth lookup, no API call,
|
||||
structured permission report). With it — the explicit operator-directed
|
||||
contaminated-PR closure path — the close proceeds and is audited as a
|
||||
distinct ``close_pr`` action carrying the required capability, so final
|
||||
reports can prove exactly which mutation capability was exercised.
|
||||
|
||||
Ordinary edits (title/body/base) and reopening never require the close
|
||||
capability: PR comment, PR edit, and PR close remain distinct capabilities.
|
||||
"""
|
||||
import sys
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
|
||||
|
||||
import mcp_server
|
||||
from mcp_server import gitea_edit_pr
|
||||
|
||||
FAKE_AUTH = "token fake"
|
||||
|
||||
AUTHOR_NO_CLOSE = {
|
||||
"profile_name": "prgs-author",
|
||||
"allowed_operations": [
|
||||
"gitea.read", "gitea.pr.create", "gitea.pr.comment",
|
||||
"gitea.branch.push", "gitea.issue.comment",
|
||||
],
|
||||
"forbidden_operations": ["gitea.pr.approve", "gitea.pr.merge"],
|
||||
"audit_label": "prgs-author",
|
||||
}
|
||||
|
||||
AUTHOR_WITH_CLOSE = {
|
||||
"profile_name": "prgs-author-closer",
|
||||
"allowed_operations": AUTHOR_NO_CLOSE["allowed_operations"] + ["gitea.pr.close"],
|
||||
"forbidden_operations": ["gitea.pr.approve", "gitea.pr.merge"],
|
||||
"audit_label": "prgs-author-closer",
|
||||
}
|
||||
|
||||
|
||||
class TestEditPrCloseGate(unittest.TestCase):
|
||||
|
||||
def setUp(self):
|
||||
self.mock_api = patch("mcp_server.api_request").start()
|
||||
self.mock_auth = patch("mcp_server.get_auth_header", return_value=FAKE_AUTH).start()
|
||||
# Deterministic permission reports: no real operator config, no switching.
|
||||
patch("gitea_config.load_config", return_value={}).start()
|
||||
patch("gitea_config.is_runtime_switching_enabled", return_value=False).start()
|
||||
patch("gitea_audit.audit_enabled", return_value=False).start()
|
||||
mcp_server._IDENTITY_CACHE.clear()
|
||||
|
||||
def tearDown(self):
|
||||
patch.stopall()
|
||||
mcp_server._IDENTITY_CACHE.clear()
|
||||
|
||||
def _set_profile(self, profile):
|
||||
patch("mcp_server.get_profile", return_value=profile).start()
|
||||
|
||||
def test_close_blocked_without_close_capability(self):
|
||||
# Author profile without gitea.pr.close: the broad edit path must not
|
||||
# be usable as an untracked close fallback.
|
||||
self._set_profile(AUTHOR_NO_CLOSE)
|
||||
res = gitea_edit_pr(pr_number=205, state="closed", remote="prgs")
|
||||
self.assertFalse(res["success"])
|
||||
self.assertFalse(res["performed"])
|
||||
self.assertEqual(res["requested_state"], "closed")
|
||||
self.assertEqual(res["required_permission"], "gitea.pr.close")
|
||||
self.assertTrue(res["reasons"])
|
||||
report = res["permission_report"]
|
||||
self.assertEqual(report["required_permission"], "gitea.pr.close")
|
||||
self.assertEqual(report["active_profile"], "prgs-author")
|
||||
self.mock_api.assert_not_called()
|
||||
self.mock_auth.assert_not_called()
|
||||
|
||||
def test_close_blocked_when_close_forbidden(self):
|
||||
profile = dict(AUTHOR_WITH_CLOSE)
|
||||
profile["forbidden_operations"] = ["gitea.pr.close"]
|
||||
self._set_profile(profile)
|
||||
res = gitea_edit_pr(pr_number=205, state="closed", remote="prgs")
|
||||
self.assertFalse(res["success"])
|
||||
self.assertIn("profile forbids 'gitea.pr.close'", res["reasons"])
|
||||
self.mock_api.assert_not_called()
|
||||
|
||||
def test_close_blocked_when_profile_unresolvable(self):
|
||||
patch("mcp_server.get_profile", side_effect=RuntimeError("no profile")).start()
|
||||
res = gitea_edit_pr(pr_number=205, state="closed", remote="prgs")
|
||||
self.assertFalse(res["success"])
|
||||
self.assertTrue(any("fail closed" in r for r in res["reasons"]))
|
||||
self.mock_api.assert_not_called()
|
||||
|
||||
def test_operator_directed_close_allowed_and_audited(self):
|
||||
# Explicit operator-directed contaminated-PR closure: the operator
|
||||
# granted gitea.pr.close, so the close proceeds and the audit trail
|
||||
# records a distinct close_pr action with the capability proof.
|
||||
self._set_profile(AUTHOR_WITH_CLOSE)
|
||||
patch("gitea_audit.audit_enabled", return_value=True).start()
|
||||
mock_write = patch("gitea_audit.write_event").start()
|
||||
|
||||
def api_side_effect(method, url, auth, payload=None):
|
||||
if method == "GET" and "/user" in url:
|
||||
return {"login": "jcwalker3"}
|
||||
if method == "PATCH" and "pulls/205" in url:
|
||||
self.assertEqual(payload["state"], "closed")
|
||||
return {
|
||||
"number": 205,
|
||||
"title": "Contaminated PR",
|
||||
"state": "closed",
|
||||
"html_url": "url",
|
||||
"body": "No issue link",
|
||||
"head": {"ref": "feat/invalid-provenance"},
|
||||
}
|
||||
return {}
|
||||
self.mock_api.side_effect = api_side_effect
|
||||
|
||||
res = gitea_edit_pr(pr_number=205, state="closed", remote="prgs")
|
||||
self.assertTrue(res["success"])
|
||||
self.assertEqual(res["state"], "closed")
|
||||
mock_write.assert_called()
|
||||
event = mock_write.call_args[0][0]
|
||||
self.assertEqual(event["action"], "close_pr")
|
||||
self.assertEqual(
|
||||
event["request_metadata"]["required_permission"], "gitea.pr.close")
|
||||
|
||||
def test_title_edit_needs_no_close_capability(self):
|
||||
# PR edit and PR close are distinct capabilities: retitling stays on
|
||||
# the ordinary edit path.
|
||||
self._set_profile(AUTHOR_NO_CLOSE)
|
||||
self.mock_api.return_value = {
|
||||
"number": 7, "title": "Renamed", "state": "open",
|
||||
"body": "", "html_url": "u"}
|
||||
res = gitea_edit_pr(pr_number=7, title="Renamed", remote="prgs")
|
||||
self.assertTrue(res["success"])
|
||||
|
||||
def test_reopen_needs_no_close_capability(self):
|
||||
self._set_profile(AUTHOR_NO_CLOSE)
|
||||
self.mock_api.return_value = {
|
||||
"number": 7, "title": "T", "state": "open",
|
||||
"body": "", "html_url": "u"}
|
||||
res = gitea_edit_pr(pr_number=7, state="open", remote="prgs")
|
||||
self.assertTrue(res["success"])
|
||||
|
||||
def test_invalid_state_fails_closed_before_auth(self):
|
||||
# A case-variant state can neither bypass the close gate nor reach
|
||||
# the API: it is rejected as pure validation, before auth.
|
||||
self._set_profile(AUTHOR_WITH_CLOSE)
|
||||
with self.assertRaises(ValueError):
|
||||
gitea_edit_pr(pr_number=7, state="CLOSED", remote="prgs")
|
||||
self.mock_api.assert_not_called()
|
||||
self.mock_auth.assert_not_called()
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
+123
-1
@@ -42,6 +42,8 @@ from mcp_server import ( # noqa: E402
|
||||
from gitea_auth import get_profile # noqa: E402
|
||||
import gitea_config # noqa: E402
|
||||
|
||||
import mcp_server
|
||||
|
||||
FAKE_AUTH = "Basic dGVzdDp0ZXN0"
|
||||
|
||||
|
||||
@@ -1776,7 +1778,8 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
|
||||
self.mock_auth = patch("mcp_server.get_auth_header", return_value=FAKE_AUTH).start()
|
||||
patch("gitea_audit.audit_enabled", return_value=True).start()
|
||||
self.mock_audit = patch("gitea_audit.write_event").start()
|
||||
patch("mcp_server.get_profile", return_value={"profile_name": "test", "allowed_operations": ["merge", "edit", "close"], "audit_label": "test", "forbidden_operations": []}).start()
|
||||
# gitea.pr.close: closing a PR via gitea_edit_pr is capability-gated (#216).
|
||||
patch("mcp_server.get_profile", return_value={"profile_name": "test", "allowed_operations": ["merge", "edit", "close", "gitea.pr.close"], "audit_label": "test", "forbidden_operations": []}).start()
|
||||
|
||||
def tearDown(self):
|
||||
patch.stopall()
|
||||
@@ -2451,3 +2454,122 @@ class TestIssueCommentPermissionSeparation(unittest.TestCase):
|
||||
"gitea.issue.comment", reviewer["allowed_operations"],
|
||||
reviewer.get("forbidden_operations", []))
|
||||
self.assertTrue(ok)
|
||||
|
||||
|
||||
class TestVerifyMutationAuthority(unittest.TestCase):
|
||||
"""In-process mutation authority (#199, refs #194).
|
||||
|
||||
The authority record lives in mcp_server._MUTATION_AUTHORITY (per
|
||||
process, reset between tests by conftest); the CLI side-channel is
|
||||
covered by the GITEA_SESSION_PROFILE_LOCK environment lock. There is no
|
||||
lock file — nothing here touches /tmp.
|
||||
"""
|
||||
|
||||
def setUp(self):
|
||||
self.patch_profile = patch("mcp_server.get_profile")
|
||||
self.mock_profile = self.patch_profile.start()
|
||||
self.patch_username = patch("mcp_server._authenticated_username")
|
||||
self.mock_username = self.patch_username.start()
|
||||
self.mock_profile.return_value = {"profile_name": "prgs-reviewer"}
|
||||
self.mock_username.return_value = "sysadmin"
|
||||
|
||||
def tearDown(self):
|
||||
self.patch_profile.stop()
|
||||
self.patch_username.stop()
|
||||
|
||||
def _authority(self, **overrides):
|
||||
data = {
|
||||
"initial_profile": "prgs-reviewer",
|
||||
"initial_identity": "sysadmin",
|
||||
"current_profile": "prgs-reviewer",
|
||||
"current_identity": "sysadmin",
|
||||
"remote": "prgs",
|
||||
"task": "review_pr",
|
||||
"role_pivot_authorized": False,
|
||||
"role_pivot_record": None,
|
||||
"pid": os.getpid(),
|
||||
}
|
||||
data.update(overrides)
|
||||
mcp_server._MUTATION_AUTHORITY = data
|
||||
|
||||
def test_missing_authority_seeds_from_live_context(self):
|
||||
# Approved preflight path (whoami → eligibility → mutation): the
|
||||
# first mutation gate seeds the authority instead of failing closed,
|
||||
# so the standard reviewer workflow keeps working.
|
||||
mcp_server._MUTATION_AUTHORITY = None
|
||||
mcp_server.verify_mutation_authority("prgs")
|
||||
seeded = mcp_server._MUTATION_AUTHORITY
|
||||
self.assertIsNotNone(seeded)
|
||||
self.assertEqual(seeded["current_profile"], "prgs-reviewer")
|
||||
self.assertEqual(seeded["current_identity"], "sysadmin")
|
||||
self.assertEqual(seeded["remote"], "prgs")
|
||||
|
||||
def test_unresolved_profile_fails_closed(self):
|
||||
self.mock_profile.return_value = {}
|
||||
mcp_server._MUTATION_AUTHORITY = None
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
mcp_server.verify_mutation_authority("prgs")
|
||||
self.assertIn("profile unresolved", str(ctx.exception))
|
||||
|
||||
def test_mismatched_remote_fails(self):
|
||||
self._authority(remote="dadeschools")
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
mcp_server.verify_mutation_authority("prgs")
|
||||
self.assertIn("does not match locked remote", str(ctx.exception))
|
||||
|
||||
def test_profile_flip_after_record_fails(self):
|
||||
# Authority was recorded as author; the active profile now resolves
|
||||
# as reviewer (e.g. an env-var flip mid-session) — refuse.
|
||||
self._authority(
|
||||
initial_profile="prgs-author",
|
||||
initial_identity="jcwalker3",
|
||||
current_profile="prgs-author",
|
||||
current_identity="jcwalker3",
|
||||
)
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
mcp_server.verify_mutation_authority("prgs")
|
||||
self.assertIn("does not match locked authority", str(ctx.exception))
|
||||
|
||||
def test_session_lock_env_mismatch_fails(self):
|
||||
# The launching session locked the environment to the author
|
||||
# profile; the active profile resolves as reviewer — side-channel
|
||||
# override rejected even with a matching in-process authority.
|
||||
self._authority()
|
||||
with patch.dict(os.environ,
|
||||
{"GITEA_SESSION_PROFILE_LOCK": "prgs-author"}):
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
mcp_server.verify_mutation_authority("prgs")
|
||||
self.assertIn("side-channel override rejected", str(ctx.exception))
|
||||
|
||||
def test_foreign_pid_authority_is_not_trusted(self):
|
||||
# An authority record from another process (fork leftovers) is
|
||||
# discarded and reseeded from the live context, never reused.
|
||||
self._authority(current_profile="prgs-author", pid=os.getpid() + 1)
|
||||
mcp_server.verify_mutation_authority("prgs")
|
||||
self.assertEqual(
|
||||
mcp_server._MUTATION_AUTHORITY["current_profile"], "prgs-reviewer"
|
||||
)
|
||||
self.assertEqual(mcp_server._MUTATION_AUTHORITY["pid"], os.getpid())
|
||||
|
||||
def test_author_to_reviewer_pivot_blocked_without_authorization(self):
|
||||
self._authority(
|
||||
initial_profile="prgs-author",
|
||||
initial_identity="jcwalker3",
|
||||
)
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
mcp_server.verify_mutation_authority("prgs", required_role="reviewer")
|
||||
self.assertIn("without authorized role pivot", str(ctx.exception))
|
||||
|
||||
def test_authorized_pivot_is_allowed(self):
|
||||
self._authority(
|
||||
initial_profile="prgs-author",
|
||||
initial_identity="jcwalker3",
|
||||
role_pivot_authorized=True,
|
||||
)
|
||||
mcp_server.verify_mutation_authority("prgs", required_role="reviewer")
|
||||
|
||||
def test_allowed_when_match(self):
|
||||
self._authority()
|
||||
with patch.dict(os.environ,
|
||||
{"GITEA_SESSION_PROFILE_LOCK": "prgs-reviewer"}):
|
||||
mcp_server.verify_mutation_authority("prgs")
|
||||
|
||||
@@ -203,5 +203,67 @@ class TestResolveTaskCapability(unittest.TestCase):
|
||||
reasons = res.get("reasons", [])
|
||||
self.assertTrue(any("author" in str(r).lower() for r in reasons) or len(reasons) > 0)
|
||||
|
||||
# Issue #216: close_pr is a first-class resolver task gated on gitea.pr.close.
|
||||
@patch("mcp_server.api_request", return_value={"login": "author-user"})
|
||||
@patch("mcp_server.get_auth_header", return_value="token author-pass")
|
||||
def test_resolve_close_pr_author_profile_without_close_blocked(self, _auth, _api):
|
||||
# Default author profile has PR create/comment but not close: the
|
||||
# close capability must never be implied by broader author ops.
|
||||
with patch.dict(os.environ, self._env("author-profile")):
|
||||
res = mcp_server.gitea_resolve_task_capability(task="close_pr", remote="prgs")
|
||||
self.assertEqual(res["requested_task"], "close_pr")
|
||||
self.assertEqual(res["required_operation_permission"], "gitea.pr.close")
|
||||
self.assertEqual(res["required_role_kind"], "author")
|
||||
self.assertFalse(res["allowed_in_current_session"])
|
||||
self.assertTrue(res["stop_required"])
|
||||
self.assertEqual(res["matching_configured_profile"], [])
|
||||
self.assertTrue(res["different_mcp_namespace_required"])
|
||||
|
||||
@patch("mcp_server.api_request", return_value={"login": "author-user"})
|
||||
@patch("mcp_server.get_auth_header", return_value="token author-pass")
|
||||
def test_resolve_close_pr_author_profile_with_close_allowed(self, _auth, _api):
|
||||
# Operator-granted close capability resolves cleanly under an author profile.
|
||||
config = json.loads(json.dumps(CONFIG_RESOLVER))
|
||||
config["profiles"]["author-closer"] = {
|
||||
"enabled": True,
|
||||
"context": "ctx",
|
||||
"role": "author",
|
||||
"username": "author-user",
|
||||
"auth": {"type": "env", "name": "GITEA_TOKEN_AUTHOR"},
|
||||
"allowed_operations": ["gitea.read", "gitea.pr.close"],
|
||||
"forbidden_operations": ["gitea.pr.approve", "gitea.pr.merge"],
|
||||
"execution_profile": "author-closer",
|
||||
}
|
||||
self._write_config(config)
|
||||
with patch.dict(os.environ, self._env("author-closer")):
|
||||
res = mcp_server.gitea_resolve_task_capability(task="close_pr", remote="prgs")
|
||||
self.assertTrue(res["allowed_in_current_session"])
|
||||
self.assertFalse(res["stop_required"])
|
||||
self.assertIn("author-closer", res["matching_configured_profile"])
|
||||
self.assertIn("ready for operations", res["exact_safe_next_action"])
|
||||
|
||||
@patch("mcp_server.api_request", return_value={"login": "reviewer-user"})
|
||||
@patch("mcp_server.get_auth_header", return_value="token reviewer-pass")
|
||||
def test_resolve_close_pr_reviewer_profile_blocked(self, _auth, _api):
|
||||
# close_pr is author-side: a reviewer profile must be told to stop.
|
||||
with patch.dict(os.environ, self._env("reviewer-profile")):
|
||||
res = mcp_server.gitea_resolve_task_capability(task="close_pr", remote="prgs")
|
||||
self.assertFalse(res["allowed_in_current_session"])
|
||||
self.assertTrue(res["stop_required"])
|
||||
self.assertEqual(res["required_role_kind"], "author")
|
||||
self.assertIn("author", res["exact_safe_next_action"].lower())
|
||||
|
||||
@patch("mcp_server.api_request", return_value={"login": "author-user"})
|
||||
@patch("mcp_server.get_auth_header", return_value="token author-pass")
|
||||
def test_close_pr_known_and_lookalike_tasks_still_fail_closed(self, _auth, _api):
|
||||
# close_pr resolves (no Unknown-task error); near-miss spellings keep
|
||||
# failing closed so no untracked close fallback can be rationalized.
|
||||
with patch.dict(os.environ, self._env("author-profile")):
|
||||
res = mcp_server.gitea_resolve_task_capability(task="close_pr", remote="prgs")
|
||||
self.assertEqual(res["required_operation_permission"], "gitea.pr.close")
|
||||
for unknown in ("close_pull_request", "close", "pr_close"):
|
||||
with self.assertRaises(ValueError):
|
||||
mcp_server.gitea_resolve_task_capability(task=unknown, remote="prgs")
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
@@ -2,6 +2,8 @@
|
||||
|
||||
Mocks api_request and credentials.
|
||||
"""
|
||||
import io
|
||||
import os
|
||||
import sys
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
@@ -27,6 +29,11 @@ FAKE_PR_DATA = {
|
||||
|
||||
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):
|
||||
@@ -35,6 +42,11 @@ class TestArgParsing(unittest.TestCase):
|
||||
|
||||
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):
|
||||
@@ -99,5 +111,55 @@ class TestAPIPayload(unittest.TestCase):
|
||||
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()
|
||||
|
||||
@@ -21,11 +21,14 @@ import unittest
|
||||
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
|
||||
|
||||
from review_proofs import ( # noqa: E402
|
||||
assess_capability_evidence,
|
||||
assess_controller_handoff,
|
||||
assess_inventory_completeness,
|
||||
assess_live_state_recheck,
|
||||
assess_review_mutation_final_report,
|
||||
assess_role_boundary,
|
||||
assess_self_review_contamination,
|
||||
assess_sweep_evidence,
|
||||
assess_validation_report,
|
||||
build_final_report,
|
||||
pr_inventory_trust_gate,
|
||||
@@ -117,6 +120,63 @@ def _good_role_boundary():
|
||||
)
|
||||
|
||||
|
||||
def _good_capability_evidence():
|
||||
return assess_capability_evidence([
|
||||
{
|
||||
"task": "review_pr",
|
||||
"allowed": True,
|
||||
"evidence_source": (
|
||||
"gitea_resolve_task_capability(review_pr) output: "
|
||||
"allowed_in_current_session=true, profile prgs-reviewer"
|
||||
),
|
||||
},
|
||||
{
|
||||
"task": "merge_pr",
|
||||
"allowed": True,
|
||||
"evidence_source": (
|
||||
"gitea_resolve_task_capability(merge_pr) output: "
|
||||
"allowed_in_current_session=true, profile prgs-reviewer"
|
||||
),
|
||||
},
|
||||
])
|
||||
|
||||
|
||||
def _good_sweep(**overrides):
|
||||
sweep = {
|
||||
"command": (
|
||||
"git diff prgs/master...HEAD | grep -inE "
|
||||
"'password|token|secret|api[_-]?key|authorization|bearer|https?://'"
|
||||
),
|
||||
"scope": "full PR diff against prgs/master",
|
||||
"clean": True,
|
||||
}
|
||||
sweep.update(overrides)
|
||||
return assess_sweep_evidence(sweep)
|
||||
|
||||
|
||||
def _good_live_state(**overrides):
|
||||
recheck = {
|
||||
"pr_state": "open",
|
||||
"pinned_head_sha": PINNED,
|
||||
"live_head_sha": PINNED,
|
||||
"pinned_base_ref": "master",
|
||||
"live_base_ref": "master",
|
||||
"blocking_change_requests": False,
|
||||
}
|
||||
recheck.update(overrides)
|
||||
return assess_live_state_recheck(recheck)
|
||||
|
||||
|
||||
def _good_role_boundary_179(**overrides):
|
||||
kwargs = {
|
||||
"task_role": "reviewer",
|
||||
"namespaces_used": ["gitea-reviewer"],
|
||||
"justification": None,
|
||||
}
|
||||
kwargs.update(overrides)
|
||||
return assess_role_boundary(**kwargs)
|
||||
|
||||
|
||||
class TestCheckoutProof(unittest.TestCase):
|
||||
"""Required behavior 1 + 2: prove HEAD == pinned PR head or stop."""
|
||||
|
||||
@@ -467,6 +527,9 @@ class TestFinalReport(unittest.TestCase):
|
||||
"identity_eligible": True,
|
||||
"merge_performed": False,
|
||||
"issue_status_verified": True,
|
||||
"capability_evidence": _good_capability_evidence(),
|
||||
"sweep": _good_sweep(),
|
||||
"live_state": _good_live_state(),
|
||||
"role_boundary": _good_role_boundary(),
|
||||
}
|
||||
kwargs.update(overrides)
|
||||
@@ -929,5 +992,209 @@ class TestPRInventoryTrustGate(unittest.TestCase):
|
||||
self.assertTrue(res["corroborated"])
|
||||
|
||||
|
||||
class TestCapabilityEvidence(unittest.TestCase):
|
||||
"""#179 gap 1: capability claims need exact evidence."""
|
||||
|
||||
def test_evidence_backed_claims_are_proven(self):
|
||||
result = _good_capability_evidence()
|
||||
self.assertTrue(result["proven"])
|
||||
self.assertEqual(result["reasons"], [])
|
||||
|
||||
def test_claim_without_evidence_source_is_not_proven(self):
|
||||
result = assess_capability_evidence([
|
||||
{"task": "review_pr", "allowed": True, "evidence_source": ""},
|
||||
])
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertTrue(any("evidence" in r.lower() for r in result["reasons"]))
|
||||
|
||||
def test_no_claims_at_all_fails_closed(self):
|
||||
result = assess_capability_evidence([])
|
||||
self.assertFalse(result["proven"])
|
||||
|
||||
def test_disallowed_task_is_not_proven(self):
|
||||
result = assess_capability_evidence([
|
||||
{
|
||||
"task": "merge_pr",
|
||||
"allowed": False,
|
||||
"evidence_source": "gitea_resolve_task_capability output",
|
||||
},
|
||||
])
|
||||
self.assertFalse(result["proven"])
|
||||
|
||||
|
||||
class TestSweepEvidence(unittest.TestCase):
|
||||
"""#179 gap 2: secret/provenance sweep must be exact."""
|
||||
|
||||
def test_exact_sweep_is_proven(self):
|
||||
result = _good_sweep()
|
||||
self.assertEqual(result["verdict"], "exact")
|
||||
self.assertTrue(result["proven"])
|
||||
|
||||
def test_vague_sweep_without_command_is_downgraded(self):
|
||||
result = _good_sweep(command="")
|
||||
self.assertEqual(result["verdict"], "vague")
|
||||
self.assertFalse(result["proven"])
|
||||
|
||||
def test_sweep_without_scope_is_downgraded(self):
|
||||
result = _good_sweep(scope="")
|
||||
self.assertEqual(result["verdict"], "vague")
|
||||
self.assertFalse(result["proven"])
|
||||
|
||||
def test_missing_sweep_fails_closed(self):
|
||||
result = assess_sweep_evidence(None)
|
||||
self.assertEqual(result["verdict"], "missing")
|
||||
self.assertFalse(result["proven"])
|
||||
|
||||
def test_unstated_result_is_downgraded(self):
|
||||
result = _good_sweep(clean=None)
|
||||
self.assertFalse(result["proven"])
|
||||
|
||||
|
||||
class TestLiveStateRecheck(unittest.TestCase):
|
||||
"""#179 gap 3: explicit pre-mutation live-state recheck."""
|
||||
|
||||
def test_clean_recheck_is_proven(self):
|
||||
result = _good_live_state()
|
||||
self.assertTrue(result["proven"])
|
||||
self.assertFalse(result["block"])
|
||||
|
||||
def test_missing_recheck_fails_closed(self):
|
||||
result = assess_live_state_recheck(None)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertTrue(result["block"])
|
||||
|
||||
def test_closed_pr_blocks(self):
|
||||
result = _good_live_state(pr_state="closed")
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertTrue(result["block"])
|
||||
|
||||
def test_moved_head_blocks(self):
|
||||
result = _good_live_state(live_head_sha=OTHER)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertTrue(any("head" in r.lower() for r in result["reasons"]))
|
||||
|
||||
def test_changed_base_blocks(self):
|
||||
result = _good_live_state(live_base_ref="develop")
|
||||
self.assertFalse(result["proven"])
|
||||
|
||||
def test_unresolved_blocking_reviews_block(self):
|
||||
result = _good_live_state(blocking_change_requests=True)
|
||||
self.assertFalse(result["proven"])
|
||||
|
||||
def test_unchecked_blocking_state_fails_closed(self):
|
||||
result = _good_live_state(blocking_change_requests=None)
|
||||
self.assertFalse(result["proven"])
|
||||
|
||||
|
||||
class TestRoleBoundary179(unittest.TestCase):
|
||||
"""#179 gap 4: reviewer flows avoid unjustified author-namespace use."""
|
||||
|
||||
def test_native_namespace_only_is_clean(self):
|
||||
result = _good_role_boundary_179()
|
||||
self.assertTrue(result["proven"])
|
||||
|
||||
def test_foreign_namespace_without_justification_is_downgraded(self):
|
||||
result = _good_role_boundary_179(
|
||||
namespaces_used=["gitea-reviewer", "gitea-author"]
|
||||
)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertTrue(any("justif" in r.lower() for r in result["reasons"]))
|
||||
|
||||
def test_foreign_namespace_with_justification_is_clean(self):
|
||||
result = _good_role_boundary_179(
|
||||
namespaces_used=["gitea-reviewer", "gitea-author"],
|
||||
justification=(
|
||||
"author namespace read-only whoami used to evidence "
|
||||
"self-review contamination status"
|
||||
),
|
||||
)
|
||||
self.assertTrue(result["proven"])
|
||||
|
||||
def test_unreported_namespaces_fail_closed(self):
|
||||
result = _good_role_boundary_179(namespaces_used=None)
|
||||
self.assertFalse(result["proven"])
|
||||
|
||||
|
||||
class TestFinalReport179Bar(unittest.TestCase):
|
||||
"""#179 acceptance adds capability, sweep, live-state, and role proofs."""
|
||||
|
||||
def _report(self, **overrides):
|
||||
kwargs = {
|
||||
"checkout_proof": _good_checkout(),
|
||||
"inventory": _good_inventory(),
|
||||
"validation": _good_validation(),
|
||||
"contamination": _good_contamination(),
|
||||
"identity_eligible": True,
|
||||
"merge_performed": False,
|
||||
"issue_status_verified": True,
|
||||
"capability_evidence": _good_capability_evidence(),
|
||||
"sweep": _good_sweep(),
|
||||
"live_state": _good_live_state(),
|
||||
"role_boundary": _good_role_boundary(),
|
||||
}
|
||||
kwargs.update(overrides)
|
||||
return build_final_report(**kwargs)
|
||||
|
||||
def test_all_179_proofs_present_is_grade_a(self):
|
||||
report = self._report()
|
||||
self.assertEqual(report["grade"], "A")
|
||||
self.assertTrue(report["capability_evidence_proven"])
|
||||
self.assertEqual(report["sweep_verdict"], "exact")
|
||||
self.assertTrue(report["live_state_recheck_proven"])
|
||||
self.assertTrue(report["role_boundary_clean"])
|
||||
|
||||
def test_missing_capability_evidence_downgrades(self):
|
||||
report = self._report(capability_evidence=None)
|
||||
self.assertNotEqual(report["grade"], "A")
|
||||
self.assertFalse(report["capability_evidence_proven"])
|
||||
|
||||
def test_unevidenced_capability_claim_downgrades(self):
|
||||
report = self._report(
|
||||
capability_evidence=assess_capability_evidence([
|
||||
{"task": "review_pr", "allowed": True, "evidence_source": ""},
|
||||
])
|
||||
)
|
||||
self.assertNotEqual(report["grade"], "A")
|
||||
|
||||
def test_vague_sweep_downgrades(self):
|
||||
report = self._report(sweep=_good_sweep(command=""))
|
||||
self.assertNotEqual(report["grade"], "A")
|
||||
self.assertEqual(report["sweep_verdict"], "vague")
|
||||
|
||||
def test_missing_sweep_downgrades(self):
|
||||
report = self._report(sweep=None)
|
||||
self.assertNotEqual(report["grade"], "A")
|
||||
|
||||
def test_missing_live_state_recheck_downgrades_and_blocks_merge(self):
|
||||
report = self._report(live_state=None)
|
||||
self.assertNotEqual(report["grade"], "A")
|
||||
self.assertFalse(report["merge_allowed"])
|
||||
self.assertFalse(report["live_state_recheck_proven"])
|
||||
|
||||
def test_stale_live_state_blocks_merge(self):
|
||||
report = self._report(live_state=_good_live_state(live_head_sha=OTHER))
|
||||
self.assertNotEqual(report["grade"], "A")
|
||||
self.assertFalse(report["merge_allowed"])
|
||||
|
||||
def test_merge_claim_without_live_recheck_is_a_violation(self):
|
||||
report = self._report(live_state=None, merge_performed=True)
|
||||
self.assertEqual(report["grade"], "blocked")
|
||||
self.assertTrue(report["violations"])
|
||||
|
||||
def test_unjustified_author_namespace_downgrades(self):
|
||||
report = self._report(
|
||||
role_boundary=_good_role_boundary_179(
|
||||
namespaces_used=["gitea-reviewer", "gitea-author"]
|
||||
)
|
||||
)
|
||||
self.assertNotEqual(report["grade"], "A")
|
||||
self.assertFalse(report["role_boundary_clean"])
|
||||
|
||||
def test_positive_baseline_from_173_still_holds(self):
|
||||
report = self._report()
|
||||
self.assertTrue(report["inventory_complete"])
|
||||
self.assertTrue(report["validated_on_pinned_head"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
@@ -0,0 +1,188 @@
|
||||
"""Tests for pre-task role/session router (#206)."""
|
||||
import json
|
||||
import os
|
||||
import sys
|
||||
import tempfile
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
|
||||
|
||||
import gitea_config
|
||||
import mcp_server
|
||||
import role_session_router
|
||||
from review_proofs import assess_role_route_handoff
|
||||
|
||||
CONFIG = {
|
||||
"version": 2,
|
||||
"contexts": {
|
||||
"ctx": {
|
||||
"enabled": True,
|
||||
"gitea": {"enabled": True, "base_url": "https://gitea.example.com"},
|
||||
}
|
||||
},
|
||||
"profiles": {
|
||||
"prgs-author": {
|
||||
"enabled": True,
|
||||
"context": "ctx",
|
||||
"role": "author",
|
||||
"username": "jcwalker3",
|
||||
"auth": {"type": "env", "name": "GITEA_TOKEN_AUTHOR"},
|
||||
"allowed_operations": [
|
||||
"gitea.read", "gitea.issue.create", "gitea.pr.create",
|
||||
"gitea.branch.push", "gitea.issue.comment",
|
||||
],
|
||||
"forbidden_operations": [
|
||||
"gitea.pr.approve", "gitea.pr.merge", "gitea.pr.review",
|
||||
],
|
||||
"execution_profile": "prgs-author",
|
||||
},
|
||||
"prgs-reviewer": {
|
||||
"enabled": True,
|
||||
"context": "ctx",
|
||||
"role": "reviewer",
|
||||
"username": "sysadmin",
|
||||
"auth": {"type": "env", "name": "GITEA_TOKEN_REVIEWER"},
|
||||
"allowed_operations": [
|
||||
"gitea.read", "gitea.pr.review", "gitea.pr.approve",
|
||||
"gitea.pr.merge", "gitea.issue.comment",
|
||||
],
|
||||
"forbidden_operations": [
|
||||
"gitea.pr.create", "gitea.branch.push", "gitea.issue.create",
|
||||
],
|
||||
"execution_profile": "prgs-reviewer",
|
||||
},
|
||||
},
|
||||
"rules": {"allow_runtime_switching": False},
|
||||
}
|
||||
|
||||
|
||||
class TestRoleSessionRouter(unittest.TestCase):
|
||||
def setUp(self):
|
||||
role_session_router.clear_route_state()
|
||||
self._remotes = patch.dict(mcp_server.REMOTES, {
|
||||
"prgs": {
|
||||
"host": "gitea.example.com",
|
||||
"org": "Scaled-Tech-Consulting",
|
||||
"repo": "Gitea-Tools",
|
||||
},
|
||||
})
|
||||
self._remotes.start()
|
||||
mcp_server._IDENTITY_CACHE.clear()
|
||||
gitea_config._active_profile_override = None
|
||||
self._dir = tempfile.TemporaryDirectory()
|
||||
self.config_path = os.path.join(self._dir.name, "profiles.json")
|
||||
with open(self.config_path, "w", encoding="utf-8") as fh:
|
||||
fh.write(json.dumps(CONFIG))
|
||||
|
||||
def tearDown(self):
|
||||
self._remotes.stop()
|
||||
role_session_router.clear_route_state()
|
||||
mcp_server._IDENTITY_CACHE.clear()
|
||||
gitea_config._active_profile_override = None
|
||||
self._dir.cleanup()
|
||||
|
||||
def _env(self, profile):
|
||||
return {
|
||||
"GITEA_MCP_CONFIG": self.config_path,
|
||||
"GITEA_MCP_PROFILE": profile,
|
||||
"GITEA_TOKEN_AUTHOR": "author-pass",
|
||||
"GITEA_TOKEN_REVIEWER": "reviewer-pass",
|
||||
}
|
||||
|
||||
def test_reviewer_task_under_author_profile_wrong_role_stop(self):
|
||||
with patch.dict(os.environ, self._env("prgs-author")):
|
||||
route = mcp_server.gitea_route_task_session(
|
||||
task_type="review_pr", remote="prgs"
|
||||
)
|
||||
self.assertEqual(route["route_result"], role_session_router.ROUTE_WRONG_ROLE)
|
||||
self.assertFalse(route["downstream_allowed"])
|
||||
self.assertIn("Wrong role/session for reviewer task", route["message"])
|
||||
|
||||
@patch("mcp_server.api_request")
|
||||
@patch("mcp_server.get_auth_header", return_value="token author-pass")
|
||||
def test_issue_creation_blocked_after_reviewer_wrong_role_stop(
|
||||
self, _auth, mock_api
|
||||
):
|
||||
with patch.dict(os.environ, self._env("prgs-author")):
|
||||
mcp_server.gitea_route_task_session(task_type="review_pr", remote="prgs")
|
||||
result = mcp_server.gitea_create_issue(
|
||||
title="process issue fallback",
|
||||
body="should not post",
|
||||
remote="prgs",
|
||||
)
|
||||
self.assertFalse(result.get("success", True))
|
||||
self.assertFalse(result.get("performed", True))
|
||||
issue_posts = [
|
||||
c for c in mock_api.call_args_list
|
||||
if c.args[0] == "POST" and str(c.args[1]).endswith("/issues")
|
||||
]
|
||||
self.assertEqual(issue_posts, [])
|
||||
|
||||
@patch("mcp_server.api_request", return_value={"number": 999})
|
||||
@patch("mcp_server.get_auth_header", return_value="token author-pass")
|
||||
def test_author_task_allowed_after_explicit_author_route(
|
||||
self, _auth, _api
|
||||
):
|
||||
with patch.dict(os.environ, self._env("prgs-author")):
|
||||
route = mcp_server.gitea_route_task_session(
|
||||
task_type="create_issue", remote="prgs"
|
||||
)
|
||||
self.assertEqual(
|
||||
route["route_result"], role_session_router.ROUTE_ALLOWED
|
||||
)
|
||||
result = mcp_server.gitea_create_issue(
|
||||
title="legit author task",
|
||||
remote="prgs",
|
||||
)
|
||||
self.assertEqual(result.get("number"), 999)
|
||||
|
||||
@patch("mcp_server.api_request", return_value={"login": "sysadmin"})
|
||||
@patch("mcp_server.get_auth_header", return_value="token reviewer-pass")
|
||||
def test_reviewer_task_under_reviewer_profile_allowed(self, _auth, _api):
|
||||
with patch.dict(os.environ, self._env("prgs-reviewer")):
|
||||
route = mcp_server.gitea_route_task_session(
|
||||
task_type="review_pr", remote="prgs"
|
||||
)
|
||||
self.assertEqual(route["route_result"], role_session_router.ROUTE_ALLOWED)
|
||||
self.assertTrue(route["downstream_allowed"])
|
||||
|
||||
@patch("mcp_server.api_request", return_value={"login": "sysadmin"})
|
||||
@patch("mcp_server.get_auth_header", return_value="token reviewer-pass")
|
||||
def test_author_task_under_reviewer_profile_routes_to_author(self, _auth, _api):
|
||||
with patch.dict(os.environ, self._env("prgs-reviewer")):
|
||||
route = mcp_server.gitea_route_task_session(
|
||||
task_type="create_issue", remote="prgs"
|
||||
)
|
||||
self.assertEqual(
|
||||
route["route_result"], role_session_router.ROUTE_TO_AUTHOR
|
||||
)
|
||||
self.assertFalse(route["downstream_allowed"])
|
||||
|
||||
def test_activate_profile_blocked_in_static_mode(self):
|
||||
with patch.dict(os.environ, self._env("prgs-author")):
|
||||
result = mcp_server.gitea_activate_profile(
|
||||
profile_name="prgs-reviewer", remote="prgs"
|
||||
)
|
||||
self.assertFalse(result["success"])
|
||||
self.assertIn("switching is disabled", result["message"])
|
||||
|
||||
def test_handoff_requires_route_fields(self):
|
||||
incomplete = assess_role_route_handoff("Task done.")
|
||||
self.assertFalse(incomplete["complete"])
|
||||
self.assertIn("Task type", incomplete["missing_fields"])
|
||||
|
||||
complete = assess_role_route_handoff(
|
||||
"\n".join([
|
||||
"Task type: review_pr",
|
||||
"Required role: reviewer",
|
||||
"Active role: author",
|
||||
"Route result: wrong_role_stop",
|
||||
]),
|
||||
route_result={"route_result": "wrong_role_stop"},
|
||||
)
|
||||
self.assertTrue(complete["complete"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user