fix(auth): route delete_branch capability to the reconciler role (Closes #729)
Remote post-merge branch deletion was blocked because the capability
resolver classified delete_branch as an author-role task while
gitea.branch.delete is granted only to prgs-reconciler. No configured
profile could satisfy both the permission gate and the role gate, so
every deletion failed closed with performed=false.
Re-home delete_branch from the author role to the reconciler role in
both single-source-of-truth maps:
- task_capability_map.TASK_CAPABILITY_MAP["delete_branch"].role
- role_session_router: TASK_REQUIRED_ROLE + AUTHOR_TASKS/RECONCILER_TASKS
resolve_task_capability("delete_branch") now resolves to prgs-reconciler.
In gitea_delete_branch, the reconciler is now the delete-capable role and
performs raw deletion through the existing reconciliation-cleanup-phase
authorization gate (operator approval + safe_to_delete proof) plus the
preservation/protected guards; the prior reconciler->cleanup redirect is
removed as it would orphan that guarded flow. Author, reviewer, and
merger stay denied by the permission gate and the required-role gate.
gitea_cleanup_merged_pr_branch remains a separate reconciler-owned path
with independent merged/ancestry/ownership verification.
Tests: reconciler resolves + performs eligible deletion; author/reviewer/
merger denied even when holding the permission; protected/preservation
branches remain fail-closed; both role maps asserted in agreement.
Updated pre-existing tests that encoded the superseded author-delete /
#687 reconciler-redirect contract. Full suite: 3190 passed, 6 skipped,
1 pre-existing unrelated failure (test_issue_702 create_pr stale-binding).
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01UracxyRHQw49rZBaexHdft
This commit is contained in:
@@ -1,9 +1,15 @@
|
||||
"""Tests for gitea_delete_branch capability gate (Issue #408).
|
||||
"""Tests for gitea_delete_branch capability + role gate (Issue #408, #729).
|
||||
|
||||
``gitea_delete_branch`` requires the exact ``gitea.branch.delete`` operation:
|
||||
without it the delete fails closed (no preflight, no auth lookup, no API call,
|
||||
structured permission report). With it, deletion proceeds through existing
|
||||
preflight and audit unchanged.
|
||||
structured permission report).
|
||||
|
||||
#729: delete_branch is reconciler-owned. ``gitea.branch.delete`` is granted only
|
||||
to the reconciler profile, so the resolver classifies delete_branch as a
|
||||
reconciler task. Raw ``gitea_delete_branch`` still redirects the reconciler to
|
||||
the guarded ``gitea_cleanup_merged_pr_branch`` path (#514/#687); author,
|
||||
reviewer, and merger remain denied by the permission gate and/or the required
|
||||
role gate.
|
||||
"""
|
||||
import json
|
||||
import os
|
||||
@@ -16,6 +22,8 @@ sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.par
|
||||
|
||||
import mcp_server
|
||||
from mcp_server import gitea_delete_branch
|
||||
import task_capability_map
|
||||
import role_session_router
|
||||
|
||||
FAKE_AUTH = "token fake"
|
||||
|
||||
@@ -38,6 +46,22 @@ AUTHOR_WITH_DELETE = {
|
||||
"audit_label": "prgs-author-deleter",
|
||||
}
|
||||
|
||||
# #729: delete_branch is reconciler-owned. The reconciler holds
|
||||
# gitea.branch.delete but raw gitea_delete_branch redirects it to the guarded
|
||||
# gitea_cleanup_merged_pr_branch path (#514/#687).
|
||||
RECONCILER_WITH_DELETE = {
|
||||
"profile_name": "prgs-reconciler",
|
||||
"role": "reconciler",
|
||||
"allowed_operations": [
|
||||
"gitea.read", "gitea.issue.comment", "gitea.pr.comment",
|
||||
"gitea.branch.delete",
|
||||
],
|
||||
"forbidden_operations": [
|
||||
"gitea.pr.approve", "gitea.pr.merge", "gitea.pr.create",
|
||||
],
|
||||
"audit_label": "prgs-reconciler",
|
||||
}
|
||||
|
||||
CONFIG = {
|
||||
"version": 2,
|
||||
"contexts": {
|
||||
@@ -81,6 +105,20 @@ CONFIG = {
|
||||
],
|
||||
"execution_profile": "reviewer-profile",
|
||||
},
|
||||
# #729: reconciler is the only delete-capable role.
|
||||
"reconciler-profile": {
|
||||
"enabled": True,
|
||||
"context": "ctx",
|
||||
"role": "reconciler",
|
||||
"username": "reconciler-user",
|
||||
"auth": {"type": "env", "name": "GITEA_TOKEN_RECONCILER"},
|
||||
"allowed_operations": [
|
||||
"gitea.read", "gitea.issue.comment", "gitea.pr.comment",
|
||||
"gitea.branch.delete",
|
||||
],
|
||||
"forbidden_operations": ["gitea.pr.create", "gitea.pr.merge"],
|
||||
"execution_profile": "reconciler-profile",
|
||||
},
|
||||
},
|
||||
"rules": {"allow_runtime_switching": False},
|
||||
}
|
||||
@@ -121,6 +159,9 @@ class TestDeleteBranchToolGate(unittest.TestCase):
|
||||
def _set_profile(self, profile):
|
||||
patch("mcp_server.get_profile", return_value=profile).start()
|
||||
|
||||
def _delete_calls(self):
|
||||
return [c for c in self.mock_api.call_args_list if c.args[0] == "DELETE"]
|
||||
|
||||
def test_blocked_without_delete_capability(self):
|
||||
self._set_profile(AUTHOR_NO_DELETE)
|
||||
res = gitea_delete_branch(branch="feat/branch", remote="prgs")
|
||||
@@ -135,33 +176,53 @@ class TestDeleteBranchToolGate(unittest.TestCase):
|
||||
self.mock_api.assert_not_called()
|
||||
self.mock_auth.assert_not_called()
|
||||
|
||||
def test_allowed_delete_proceeds(self):
|
||||
def test_author_with_delete_perm_denied_by_role_gate(self):
|
||||
"""#729: even an author holding gitea.branch.delete is denied — the
|
||||
required role is now reconciler. Fail closed, no API DELETE."""
|
||||
self._set_profile(AUTHOR_WITH_DELETE)
|
||||
mcp_server.record_preflight_check("whoami")
|
||||
mcp_server.record_preflight_check("capability", resolved_role="author")
|
||||
res = gitea_delete_branch(branch="feat/branch", remote="prgs")
|
||||
self.assertFalse(res["success"])
|
||||
self.assertFalse(res["performed"])
|
||||
self.assertEqual(res["required_role_kind"], "reconciler")
|
||||
self.assertEqual(res["active_role_kind"], "author")
|
||||
self.assertTrue(res["reasons"])
|
||||
self.assertFalse(self._delete_calls())
|
||||
|
||||
def test_reviewer_with_delete_perm_denied_by_role_gate(self):
|
||||
"""A reviewer that somehow holds the permission is still denied."""
|
||||
reviewer_with_delete = {
|
||||
"profile_name": "prgs-reviewer",
|
||||
"role": "reviewer",
|
||||
"allowed_operations": [
|
||||
"gitea.read", "gitea.pr.review", "gitea.branch.delete",
|
||||
],
|
||||
"forbidden_operations": [],
|
||||
"audit_label": "prgs-reviewer",
|
||||
}
|
||||
self._set_profile(reviewer_with_delete)
|
||||
mcp_server.record_preflight_check("whoami")
|
||||
mcp_server.record_preflight_check("capability", resolved_role="reviewer")
|
||||
res = gitea_delete_branch(branch="feat/branch", remote="prgs")
|
||||
self.assertFalse(res["success"])
|
||||
self.assertFalse(res["performed"])
|
||||
self.assertEqual(res["required_role_kind"], "reconciler")
|
||||
self.assertEqual(res["active_role_kind"], "reviewer")
|
||||
self.assertFalse(self._delete_calls())
|
||||
|
||||
def test_reconciler_performs_raw_delete(self):
|
||||
"""#729: the reconciler is the delete-capable role and performs the raw
|
||||
deletion (no audit phase active here); the API DELETE is issued."""
|
||||
self._set_profile(RECONCILER_WITH_DELETE)
|
||||
mcp_server.record_preflight_check("whoami")
|
||||
mcp_server.record_preflight_check(
|
||||
"capability", resolved_role="reconciler")
|
||||
self.mock_api.return_value = {}
|
||||
res = gitea_delete_branch(branch="feat/branch", remote="prgs")
|
||||
self.assertTrue(res["success"])
|
||||
self.assertIn("deleted", res["message"])
|
||||
delete_calls = [
|
||||
c for c in self.mock_api.call_args_list if c.args[0] == "DELETE"
|
||||
]
|
||||
self.assertTrue(delete_calls)
|
||||
|
||||
def test_allowed_delete_audited_with_capability_proof(self):
|
||||
self._set_profile(AUTHOR_WITH_DELETE)
|
||||
patch("gitea_audit.audit_enabled", return_value=True).start()
|
||||
mock_write = patch("gitea_audit.write_event").start()
|
||||
mcp_server.record_preflight_check("whoami")
|
||||
mcp_server.record_preflight_check("capability", resolved_role="author")
|
||||
self.mock_api.return_value = {}
|
||||
gitea_delete_branch(branch="feat/branch", remote="prgs")
|
||||
mock_write.assert_called()
|
||||
event = mock_write.call_args[0][0]
|
||||
self.assertEqual(event["action"], "delete_branch")
|
||||
self.assertEqual(
|
||||
event["request_metadata"]["required_permission"],
|
||||
"gitea.branch.delete",
|
||||
)
|
||||
self.assertTrue(self._delete_calls())
|
||||
|
||||
|
||||
class TestDeleteBranchResolverParity(unittest.TestCase):
|
||||
@@ -194,24 +255,89 @@ class TestDeleteBranchResolverParity(unittest.TestCase):
|
||||
"GITEA_MCP_PROFILE": profile,
|
||||
"GITEA_TOKEN_AUTHOR": "author-pass",
|
||||
"GITEA_TOKEN_REVIEWER": "reviewer-pass",
|
||||
"GITEA_TOKEN_RECONCILER": "reconciler-pass",
|
||||
}
|
||||
|
||||
def _delete_calls(self):
|
||||
return [c for c in self.mock_api.call_args_list if c.args[0] == "DELETE"]
|
||||
|
||||
def test_reconciler_resolver_allows_delete_branch(self):
|
||||
"""#729: resolve(delete_branch) on the reconciler profile is allowed and
|
||||
classified as a reconciler task."""
|
||||
with patch.dict(os.environ, self._env("reconciler-profile"), clear=True):
|
||||
resolve = mcp_server.gitea_resolve_task_capability(
|
||||
task="delete_branch", remote="prgs")
|
||||
# Deterministic role-map outcomes (avoid runtime-staleness-dependent
|
||||
# allowed_in_current_session, which folds in reconnect state).
|
||||
self.assertEqual(resolve["required_role_kind"], "reconciler")
|
||||
self.assertEqual(
|
||||
resolve["required_operation_permission"], "gitea.branch.delete")
|
||||
self.assertTrue(resolve["active_profile_permission_allowed"])
|
||||
self.assertTrue(resolve["configured"])
|
||||
self.assertIn("reconciler-profile", resolve["matching_configured_profile"])
|
||||
self.assertEqual(resolve["active_role_kind"], "reconciler")
|
||||
|
||||
def test_author_resolver_denies_delete_branch(self):
|
||||
"""#729: an author session cannot resolve delete_branch — required role
|
||||
is reconciler and the author lacks the permission."""
|
||||
with patch.dict(os.environ, self._env("author-no-delete"), clear=True):
|
||||
resolve = mcp_server.gitea_resolve_task_capability(
|
||||
task="delete_branch", remote="prgs")
|
||||
self.assertEqual(resolve["required_role_kind"], "reconciler")
|
||||
self.assertFalse(resolve["allowed_in_current_session"])
|
||||
|
||||
def test_reviewer_resolver_denial_blocks_raw_tool(self):
|
||||
with patch.dict(os.environ, self._env("reviewer-profile"), clear=True):
|
||||
resolve = mcp_server.gitea_resolve_task_capability(
|
||||
task="delete_branch", remote="prgs")
|
||||
self.assertFalse(resolve["allowed_in_current_session"])
|
||||
patch("mcp_server.get_profile", return_value={
|
||||
"profile_name": "prgs-reviewer",
|
||||
"role": "reviewer",
|
||||
"allowed_operations": ["gitea.read", "gitea.pr.review"],
|
||||
"forbidden_operations": ["gitea.branch.delete"],
|
||||
}).start()
|
||||
res = gitea_delete_branch(branch="feat/branch", remote="prgs")
|
||||
self.assertFalse(res["success"])
|
||||
self.assertEqual(
|
||||
res["permission_report"]["missing_permission"],
|
||||
resolve["required_operation_permission"],
|
||||
)
|
||||
delete_calls = [
|
||||
c for c in self.mock_api.call_args_list if c.args[0] == "DELETE"
|
||||
]
|
||||
self.assertFalse(delete_calls)
|
||||
self.assertFalse(self._delete_calls())
|
||||
|
||||
|
||||
class TestDeleteBranchRoleMapParity(unittest.TestCase):
|
||||
"""#729: both single-source-of-truth maps must classify delete_branch as
|
||||
reconciler and stay in agreement."""
|
||||
|
||||
def test_task_capability_map_role_reconciler(self):
|
||||
self.assertEqual(
|
||||
task_capability_map.required_role("delete_branch"), "reconciler")
|
||||
self.assertEqual(
|
||||
task_capability_map.required_permission("delete_branch"),
|
||||
"gitea.branch.delete",
|
||||
)
|
||||
|
||||
def test_router_required_role_reconciler(self):
|
||||
self.assertEqual(
|
||||
role_session_router.TASK_REQUIRED_ROLE["delete_branch"],
|
||||
"reconciler",
|
||||
)
|
||||
self.assertEqual(
|
||||
role_session_router.required_role_for_task("delete_branch"),
|
||||
"reconciler",
|
||||
)
|
||||
|
||||
def test_router_set_membership_moved(self):
|
||||
self.assertIn("delete_branch", role_session_router.RECONCILER_TASKS)
|
||||
self.assertNotIn("delete_branch", role_session_router.AUTHOR_TASKS)
|
||||
|
||||
def test_maps_agree(self):
|
||||
self.assertEqual(
|
||||
task_capability_map.required_role("delete_branch"),
|
||||
role_session_router.TASK_REQUIRED_ROLE["delete_branch"],
|
||||
)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
unittest.main()
|
||||
|
||||
Reference in New Issue
Block a user