Clarify and enforce reviewer-vs-merger role boundaries (#483)
This commit is contained in:
@@ -43,9 +43,19 @@ CONFIG_SWITCHING_DISABLED = {
|
||||
"role": "reviewer",
|
||||
"username": "reviewer-user",
|
||||
"auth": {"type": "env", "name": "GITEA_TOKEN_REVIEWER"},
|
||||
"allowed_operations": ["gitea.read", "gitea.pr.approve", "gitea.pr.merge"],
|
||||
"forbidden_operations": ["gitea.pr.create", "gitea.branch.push"],
|
||||
"allowed_operations": ["gitea.read", "gitea.pr.review", "gitea.pr.approve"],
|
||||
"forbidden_operations": ["gitea.pr.create", "gitea.branch.push", "gitea.pr.merge"],
|
||||
"execution_profile": "reviewer-profile"
|
||||
},
|
||||
"merger-profile": {
|
||||
"enabled": True,
|
||||
"context": "ctx",
|
||||
"role": "merger",
|
||||
"username": "merger-user",
|
||||
"auth": {"type": "env", "name": "GITEA_TOKEN_MERGER"},
|
||||
"allowed_operations": ["gitea.read", "gitea.pr.merge"],
|
||||
"forbidden_operations": ["gitea.pr.create", "gitea.branch.push", "gitea.pr.approve"],
|
||||
"execution_profile": "merger-profile"
|
||||
}
|
||||
},
|
||||
"rules": {
|
||||
@@ -90,6 +100,7 @@ class TestRuntimeClarity(unittest.TestCase):
|
||||
"GITEA_MCP_REVEAL_ENDPOINTS": reveal,
|
||||
"GITEA_TOKEN_AUTHOR": "author-pass",
|
||||
"GITEA_TOKEN_REVIEWER": "reviewer-pass",
|
||||
"GITEA_TOKEN_MERGER": "merger-pass",
|
||||
}
|
||||
|
||||
# -------------------------------------------------------------------------
|
||||
@@ -125,7 +136,7 @@ class TestRuntimeClarity(unittest.TestCase):
|
||||
t for t in caps["task_capabilities"] if t["task"] == "merge_pr"
|
||||
)
|
||||
self.assertFalse(merge_entry["allowed_in_current_session"])
|
||||
self.assertIn("reviewer-profile", merge_entry["matching_configured_profiles"])
|
||||
self.assertIn("merger-profile", merge_entry["matching_configured_profiles"])
|
||||
|
||||
@patch(
|
||||
"mcp_server.assess_preflight_status",
|
||||
@@ -144,6 +155,30 @@ class TestRuntimeClarity(unittest.TestCase):
|
||||
caps = ctx["session_capabilities"]
|
||||
self.assertFalse(caps["can_author_prs"])
|
||||
self.assertFalse(caps["can_create_issues"])
|
||||
self.assertTrue(caps["can_review_prs"])
|
||||
self.assertFalse(caps["can_merge_prs"])
|
||||
merge_entry = next(
|
||||
t for t in caps["task_capabilities"] if t["task"] == "merge_pr"
|
||||
)
|
||||
self.assertFalse(merge_entry["allowed_in_current_session"])
|
||||
|
||||
@patch(
|
||||
"mcp_server.assess_preflight_status",
|
||||
return_value={"preflight_ready": True, "preflight_block_reasons": []},
|
||||
)
|
||||
@patch("mcp_server.api_request", return_value={"login": "merger-user"})
|
||||
@patch("mcp_server.get_auth_header", return_value="token merger-pass")
|
||||
def test_get_runtime_context_merger(self, _auth, _api, _preflight):
|
||||
with patch.dict(os.environ, self._env("merger-profile"), clear=True):
|
||||
ctx = mcp_server.gitea_get_runtime_context(remote="dadeschools")
|
||||
self.assertEqual(ctx["active_profile"], "merger-profile")
|
||||
self.assertEqual(ctx["authenticated_username"], "merger-user")
|
||||
self.assertTrue(ctx["review_merge_allowed"])
|
||||
self.assertEqual(ctx["suggested_fix"], "none")
|
||||
self.assertEqual(ctx["safe_next_action"], "None; ready for operations.")
|
||||
caps = ctx["session_capabilities"]
|
||||
self.assertFalse(caps["can_author_prs"])
|
||||
self.assertFalse(caps["can_create_issues"])
|
||||
self.assertFalse(caps["can_review_prs"])
|
||||
self.assertTrue(caps["can_merge_prs"])
|
||||
merge_entry = next(
|
||||
@@ -160,7 +195,7 @@ class TestRuntimeClarity(unittest.TestCase):
|
||||
with patch.dict(os.environ, self._env("author-profile", reveal="0"), clear=True):
|
||||
res = mcp_server.gitea_list_profiles()
|
||||
profiles = res["profiles"]
|
||||
self.assertEqual(len(profiles), 2)
|
||||
self.assertEqual(len(profiles), 3)
|
||||
|
||||
author_prof = next(p for p in profiles if p["name"] == "author-profile")
|
||||
self.assertTrue(author_prof["is_active"])
|
||||
@@ -176,6 +211,13 @@ class TestRuntimeClarity(unittest.TestCase):
|
||||
self.assertEqual(reviewer_prof["base_url"], "<redacted>")
|
||||
self.assertEqual(reviewer_prof["identity_status"], "credentials present")
|
||||
|
||||
merger_prof = next(p for p in profiles if p["name"] == "merger-profile")
|
||||
self.assertFalse(merger_prof["is_active"])
|
||||
self.assertEqual(merger_prof["role_kind"], "merger")
|
||||
self.assertEqual(merger_prof["auth"]["name"], "<redacted>")
|
||||
self.assertEqual(merger_prof["base_url"], "<redacted>")
|
||||
self.assertEqual(merger_prof["identity_status"], "credentials present")
|
||||
|
||||
@patch("mcp_server.api_request", return_value={"login": "author-user"})
|
||||
@patch("mcp_server.get_auth_header", return_value="token author-pass")
|
||||
def test_list_profiles_revealed_under_opt_in(self, _auth, _api):
|
||||
|
||||
Reference in New Issue
Block a user