fix(auth): route delete_branch capability to the reconciler role (Closes #729) #730

Merged
jcwalker3 merged 1 commits from fix/issue-729-delete-branch-reconciler-role into master 2026-07-17 18:59:20 -05:00
7 changed files with 206 additions and 75 deletions
+9 -22
View File
@@ -8559,31 +8559,18 @@ def gitea_delete_branch(
} }
# Possessing gitea.branch.delete alone is not enough for arbitrary deletion. # Possessing gitea.branch.delete alone is not enough for arbitrary deletion.
# task_capability_map maps delete_branch → author; reconciler must use the # #729: task_capability_map maps delete_branch → reconciler (only the
# guarded cleanup_merged_pr_branch path only (#687 / #514). # reconciler profile holds gitea.branch.delete). The reconciler performs raw
# deletion through the reconciliation-cleanup-phase authorization gate below
# (operator approval + safe_to_delete proof), which is the sanctioned
# authorized-cleanup role for raw branch deletion (#514/#687). Author,
# reviewer, and merger are denied by the permission gate above and, even if
# they somehow hold the permission, by the required-role gate here.
# gitea_cleanup_merged_pr_branch remains a separate reconciler-owned path
# with independent merged/ancestry/ownership verification.
profile = get_profile() profile = get_profile()
active_role = _profile_role_kind(profile) active_role = _profile_role_kind(profile)
required_role = task_capability_map.required_role("delete_branch") required_role = task_capability_map.required_role("delete_branch")
if active_role == "reconciler":
return {
"success": False,
"performed": False,
"required_permission": "gitea.branch.delete",
"required_role_kind": required_role,
"active_role_kind": active_role,
"reasons": [
"reconciler profile cannot use raw gitea_delete_branch; "
"use gitea_cleanup_merged_pr_branch for a fully merged PR "
"source branch only (fail closed)"
],
"exact_next_action": (
"Call gitea_cleanup_merged_pr_branch with pr_number, the "
"exact PR head branch, and confirmation "
"'CLEANUP MERGED PR <n> BRANCH <branch>' after capability "
"resolve for cleanup_merged_pr_branch."
),
"permission_report": _permission_block_report("gitea.branch.delete"),
}
if active_role != required_role: if active_role != required_role:
return { return {
"success": False, "success": False,
+6 -2
View File
@@ -76,7 +76,6 @@ AUTHOR_TASKS = frozenset({
"create_pr", "create_pr",
"comment_pr", "comment_pr",
"address_pr_change_requests", "address_pr_change_requests",
"delete_branch",
"work_issue", "work_issue",
"work-issue", "work-issue",
"reconcile_landed_pr", "reconcile_landed_pr",
@@ -84,6 +83,10 @@ AUTHOR_TASKS = frozenset({
RECONCILER_TASKS = frozenset({ RECONCILER_TASKS = frozenset({
"cleanup_merged_pr_branch", "cleanup_merged_pr_branch",
# #729: delete_branch is reconciler-owned (gitea.branch.delete is granted
# only to the reconciler profile). Raw gitea_delete_branch redirects here to
# the guarded gitea_cleanup_merged_pr_branch path (#514/#687).
"delete_branch",
"reconcile_already_landed_pr", "reconcile_already_landed_pr",
"reconcile_already_landed", "reconcile_already_landed",
"reconcile-landed-pr", "reconcile-landed-pr",
@@ -99,7 +102,8 @@ TASK_REQUIRED_ROLE = {
"create_pr": "author", "create_pr": "author",
"comment_pr": "author", "comment_pr": "author",
"address_pr_change_requests": "author", "address_pr_change_requests": "author",
"delete_branch": "author", # #729: reconciler-owned; see RECONCILER_TASKS and task_capability_map.
"delete_branch": "reconciler",
"review_pr": "reviewer", "review_pr": "reviewer",
"merge_pr": "merger", "merge_pr": "merger",
"blind_pr_queue_review": "reviewer", "blind_pr_queue_review": "reviewer",
+7 -1
View File
@@ -221,9 +221,15 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = {
"permission": "gitea.pr.merge", "permission": "gitea.pr.merge",
"role": "merger", "role": "merger",
}, },
# #729: delete_branch is reconciler-owned. gitea.branch.delete is granted
# only to the reconciler profile, so the resolver must classify this task as
# reconciler (previously "author", which no delete-capable profile held).
# Raw gitea_delete_branch still redirects reconciler to the guarded
# gitea_cleanup_merged_pr_branch path (#514/#687); author/reviewer/merger
# stay denied by both the permission gate and this role gate.
"delete_branch": { "delete_branch": {
"permission": "gitea.branch.delete", "permission": "gitea.branch.delete",
"role": "author", "role": "reconciler",
}, },
"cleanup_merged_pr_branch": { "cleanup_merged_pr_branch": {
"permission": "gitea.branch.delete", "permission": "gitea.branch.delete",
+5 -4
View File
@@ -26,8 +26,9 @@ from review_proofs import assess_audit_reconciliation_report as proofs_assess
from task_capability_map import required_permission, required_role from task_capability_map import required_permission, required_role
DELETE_PROFILE = { DELETE_PROFILE = {
"profile_name": "prgs-author-delete", # #729: delete_branch is reconciler-owned.
"role": "author", "profile_name": "prgs-reconciler-delete",
"role": "reconciler",
"allowed_operations": [ "allowed_operations": [
"gitea.read", "gitea.read",
"gitea.pr.create", "gitea.pr.create",
@@ -238,7 +239,7 @@ class TestMcpGates(unittest.TestCase):
@patch("mcp_server.get_profile", return_value=DELETE_PROFILE) @patch("mcp_server.get_profile", return_value=DELETE_PROFILE)
def test_delete_branch_blocked_in_audit_phase(self, _profile): def test_delete_branch_blocked_in_audit_phase(self, _profile):
mcp_server.record_preflight_check("whoami") mcp_server.record_preflight_check("whoami")
mcp_server.record_preflight_check("capability", resolved_role="author") mcp_server.record_preflight_check("capability", resolved_role="reconciler")
result = mcp_server.gitea_delete_branch(branch="feat/dup", remote="prgs") result = mcp_server.gitea_delete_branch(branch="feat/dup", remote="prgs")
self.assertFalse(result["success"]) self.assertFalse(result["success"])
self.assertEqual(result["audit_phase"], PHASE_AUDIT) self.assertEqual(result["audit_phase"], PHASE_AUDIT)
@@ -278,7 +279,7 @@ class TestMcpGates(unittest.TestCase):
after_state="gone", after_state="gone",
) )
mcp_server.record_preflight_check("whoami") mcp_server.record_preflight_check("whoami")
mcp_server.record_preflight_check("capability", resolved_role="author") mcp_server.record_preflight_check("capability", resolved_role="reconciler")
self.mock_api.return_value = {} self.mock_api.return_value = {}
result = mcp_server.gitea_delete_branch(branch="feat/dup", remote="prgs") result = mcp_server.gitea_delete_branch(branch="feat/dup", remote="prgs")
self.assertTrue(result["success"]) self.assertTrue(result["success"])
+20 -14
View File
@@ -257,22 +257,27 @@ class TestMergedPrBranchCleanupTool(unittest.TestCase):
] ]
self.assertFalse(delete_calls) self.assertFalse(delete_calls)
def test_reconciler_with_branch_delete_cannot_raw_delete(self): def test_reconciler_with_branch_delete_can_raw_delete(self):
"""#687: reconciler + gitea.branch.delete still cannot call raw delete.""" """#729: delete_branch is re-homed from author to reconciler. The
reconciler is the delete-capable role and performs raw deletion of an
eligible (non-preservation, non-protected) branch — superseding the
#687 redirect that previously blocked it."""
patch( patch(
"mcp_server.get_profile", "mcp_server.get_profile",
return_value=dict(RECONCILER_WITH_DELETE), return_value=dict(RECONCILER_WITH_DELETE),
).start() ).start()
mcp_server.record_preflight_check("whoami")
mcp_server.record_preflight_check("capability", resolved_role="reconciler")
self.mock_api.return_value = {}
res = gitea_delete_branch( res = gitea_delete_branch(
branch="fix/issue-683-workflow-guard-hardening", branch="fix/issue-683-workflow-guard-hardening",
remote="prgs", remote="prgs",
) )
self.assertFalse(res.get("success", True)) self.assertTrue(res.get("success"))
self.assertFalse(res.get("performed", True)) delete_calls = [
reasons = " ".join(res.get("reasons") or []) call for call in self.mock_api.call_args_list if call.args[0] == "DELETE"
self.assertIn("raw gitea_delete_branch", reasons) ]
self.assertIn("cleanup_merged_pr_branch", reasons) self.assertTrue(delete_calls)
self.mock_api.assert_not_called()
def test_reconciler_raw_delete_denies_preservation_branch(self): def test_reconciler_raw_delete_denies_preservation_branch(self):
patch( patch(
@@ -286,17 +291,18 @@ class TestMergedPrBranchCleanupTool(unittest.TestCase):
self.assertFalse(res.get("performed", True)) self.assertFalse(res.get("performed", True))
self.mock_api.assert_not_called() self.mock_api.assert_not_called()
def test_author_with_branch_delete_role_ok_but_preserve_blocked(self): def test_reconciler_raw_delete_role_ok_but_preserve_blocked(self):
"""Author role may use raw delete path when permitted; preserve fails closed.""" """#729: reconciler role may use raw delete path when permitted;
preservation branch still fails closed."""
patch( patch(
"mcp_server.get_profile", "mcp_server.get_profile",
return_value={ return_value={
"profile_name": "prgs-author", "profile_name": "prgs-reconciler",
"role": "author", "role": "reconciler",
"allowed_operations": [ "allowed_operations": [
"gitea.read", "gitea.read",
"gitea.pr.create", "gitea.issue.comment",
"gitea.branch.push", "gitea.pr.comment",
"gitea.branch.delete", "gitea.branch.delete",
], ],
"forbidden_operations": ["gitea.pr.approve", "gitea.pr.merge"], "forbidden_operations": ["gitea.pr.approve", "gitea.pr.merge"],
+154 -28
View File
@@ -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: ``gitea_delete_branch`` requires the exact ``gitea.branch.delete`` operation:
without it the delete fails closed (no preflight, no auth lookup, no API call, without it the delete fails closed (no preflight, no auth lookup, no API call,
structured permission report). With it, deletion proceeds through existing structured permission report).
preflight and audit unchanged.
#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 json
import os import os
@@ -16,6 +22,8 @@ sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.par
import mcp_server import mcp_server
from mcp_server import gitea_delete_branch from mcp_server import gitea_delete_branch
import task_capability_map
import role_session_router
FAKE_AUTH = "token fake" FAKE_AUTH = "token fake"
@@ -38,6 +46,22 @@ AUTHOR_WITH_DELETE = {
"audit_label": "prgs-author-deleter", "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 = { CONFIG = {
"version": 2, "version": 2,
"contexts": { "contexts": {
@@ -81,6 +105,20 @@ CONFIG = {
], ],
"execution_profile": "reviewer-profile", "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}, "rules": {"allow_runtime_switching": False},
} }
@@ -121,6 +159,9 @@ class TestDeleteBranchToolGate(unittest.TestCase):
def _set_profile(self, profile): def _set_profile(self, profile):
patch("mcp_server.get_profile", return_value=profile).start() 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): def test_blocked_without_delete_capability(self):
self._set_profile(AUTHOR_NO_DELETE) self._set_profile(AUTHOR_NO_DELETE)
res = gitea_delete_branch(branch="feat/branch", remote="prgs") 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_api.assert_not_called()
self.mock_auth.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) self._set_profile(AUTHOR_WITH_DELETE)
mcp_server.record_preflight_check("whoami") mcp_server.record_preflight_check("whoami")
mcp_server.record_preflight_check("capability", resolved_role="author") mcp_server.record_preflight_check("capability", resolved_role="author")
res = gitea_delete_branch(branch="feat/branch", remote="prgs") 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.assertTrue(res["success"])
self.assertIn("deleted", res["message"]) self.assertIn("deleted", res["message"])
delete_calls = [ self.assertTrue(self._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",
)
class TestDeleteBranchResolverParity(unittest.TestCase): class TestDeleteBranchResolverParity(unittest.TestCase):
@@ -194,23 +255,88 @@ class TestDeleteBranchResolverParity(unittest.TestCase):
"GITEA_MCP_PROFILE": profile, "GITEA_MCP_PROFILE": profile,
"GITEA_TOKEN_AUTHOR": "author-pass", "GITEA_TOKEN_AUTHOR": "author-pass",
"GITEA_TOKEN_REVIEWER": "reviewer-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): def test_reviewer_resolver_denial_blocks_raw_tool(self):
with patch.dict(os.environ, self._env("reviewer-profile"), clear=True): with patch.dict(os.environ, self._env("reviewer-profile"), clear=True):
resolve = mcp_server.gitea_resolve_task_capability( resolve = mcp_server.gitea_resolve_task_capability(
task="delete_branch", remote="prgs") task="delete_branch", remote="prgs")
self.assertFalse(resolve["allowed_in_current_session"]) 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") res = gitea_delete_branch(branch="feat/branch", remote="prgs")
self.assertFalse(res["success"]) self.assertFalse(res["success"])
self.assertEqual( self.assertEqual(
res["permission_report"]["missing_permission"], res["permission_report"]["missing_permission"],
resolve["required_operation_permission"], resolve["required_operation_permission"],
) )
delete_calls = [ self.assertFalse(self._delete_calls())
c for c in self.mock_api.call_args_list if c.args[0] == "DELETE"
]
self.assertFalse(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__": if __name__ == "__main__":
+4 -3
View File
@@ -1471,8 +1471,9 @@ class TestReviewPR(unittest.TestCase):
class TestDeleteBranch(unittest.TestCase): class TestDeleteBranch(unittest.TestCase):
DELETE_PROFILE = { DELETE_PROFILE = {
"profile_name": "test-author-deleter", # #729: delete_branch is reconciler-owned.
"role": "author", "profile_name": "test-reconciler-deleter",
"role": "reconciler",
"allowed_operations": [ "allowed_operations": [
"gitea.read", "gitea.read",
"gitea.pr.create", "gitea.pr.create",
@@ -1488,7 +1489,7 @@ class TestDeleteBranch(unittest.TestCase):
@patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH)
def test_delete_branch(self, _auth, mock_api, _profile): def test_delete_branch(self, _auth, mock_api, _profile):
mcp_server.record_preflight_check("whoami") mcp_server.record_preflight_check("whoami")
mcp_server.record_preflight_check("capability", resolved_role="author") mcp_server.record_preflight_check("capability", resolved_role="reconciler")
mock_api.return_value = {} mock_api.return_value = {}
result = gitea_delete_branch(branch="feat/branch") result = gitea_delete_branch(branch="feat/branch")
self.assertTrue(result["success"]) self.assertTrue(result["success"])