Merge pull request 'Clarify and enforce reviewer-vs-merger role boundaries (#483)' (#486) from feat/issue-483-role-boundaries into master
This commit was merged in pull request #486.
This commit is contained in:
+58
-24
@@ -137,13 +137,13 @@ def verify_mutation_authority(remote: str | None, host: str | None = None,
|
||||
)
|
||||
|
||||
# Reviewer/author role pivot boundary: only an authorized pivot
|
||||
# (recorded by gitea_activate_profile) may cross author → reviewer.
|
||||
if (required_role == "reviewer"
|
||||
# (recorded by gitea_activate_profile) may cross author -> reviewer/merger.
|
||||
if (required_role in ("reviewer", "merger")
|
||||
and "author" in str(data.get("initial_profile")).lower()
|
||||
and "reviewer" in str(active_profile).lower()):
|
||||
and ("reviewer" in str(active_profile).lower() or "merger" in str(active_profile).lower())):
|
||||
if not data.get("role_pivot_authorized"):
|
||||
raise RuntimeError(
|
||||
"Attempted reviewer mutation from author session without "
|
||||
f"Attempted {required_role} mutation from author session without "
|
||||
"authorized role pivot (fail closed)"
|
||||
)
|
||||
|
||||
@@ -562,7 +562,7 @@ def _enforce_branches_only_author_mutation(worktree_path: str | None = None) ->
|
||||
|
||||
Reviewer, merger, and reconciler roles are exempt: reconciler ``close_pr``
|
||||
is a Gitea metadata mutation and must not require ``GITEA_AUTHOR_WORKTREE``
|
||||
(#468). Non-author namespaces use dedicated workspace env vars (#510).
|
||||
(#468, #483). Non-author namespaces use dedicated workspace env vars (#510).
|
||||
|
||||
The exemption honours BOTH the effective workspace role and the actual
|
||||
profile role (#540). ``comment_issue`` preflight stamps
|
||||
@@ -2497,13 +2497,23 @@ def gitea_check_pr_eligibility(
|
||||
# ("gitea.pr.merge") always match each other and never cross services.
|
||||
allowed = profile["allowed_operations"]
|
||||
forbidden = profile["forbidden_operations"]
|
||||
op_ok, op_reason = gitea_config.check_operation(action, allowed, forbidden)
|
||||
active_role = _role_kind(allowed, forbidden)
|
||||
if action == "merge" and active_role == "reviewer":
|
||||
op_ok = False
|
||||
op_reason = "reviewer-cannot-merge"
|
||||
else:
|
||||
op_ok, op_reason = gitea_config.check_operation(action, allowed, forbidden)
|
||||
|
||||
if not op_ok:
|
||||
if op_reason == "no-allowed-operations":
|
||||
reasons.append(
|
||||
"profile has no configured allowed operations (fail closed)")
|
||||
elif op_reason == "forbidden":
|
||||
reasons.append(f"profile forbids '{action}'")
|
||||
elif op_reason == "reviewer-cannot-merge":
|
||||
reasons.append(
|
||||
"Reviewer profile cannot merge. Use a merger profile/session after formal approval at current head."
|
||||
)
|
||||
elif op_reason == "invalid-forbidden-entry":
|
||||
reasons.append(
|
||||
"profile has an unrecognized forbidden operation entry "
|
||||
@@ -2527,21 +2537,23 @@ def gitea_check_pr_eligibility(
|
||||
missing_perm)
|
||||
|
||||
req_profile = None
|
||||
role_noun = "reviewer"
|
||||
if action in ("approve", "request_changes", "review"):
|
||||
req_profile = "A profile with reviewer role permissions (allowing approve/merge/review, and forbidding author operations)"
|
||||
req_profile = "A profile with reviewer role permissions (allowing approve/review, and forbidding author operations)"
|
||||
elif action == "merge":
|
||||
req_profile = "A profile with reviewer role permissions and explicit merge permission"
|
||||
req_profile = "A profile with merger role permissions (allowing merge, and forbidding author operations)"
|
||||
role_noun = "merger"
|
||||
result["required_profile"] = req_profile
|
||||
|
||||
switching_supported = gitea_config.is_runtime_switching_enabled()
|
||||
if switching_supported:
|
||||
result["fixable_by_profile_switch"] = True
|
||||
result["requires_different_namespace"] = False
|
||||
safe_step = f"Switch to a reviewer profile by calling gitea_activate_profile with a profile that allows {action}."
|
||||
safe_step = f"Switch to a {role_noun} profile by calling gitea_activate_profile with a profile that allows {action}."
|
||||
else:
|
||||
result["fixable_by_profile_switch"] = False
|
||||
result["requires_different_namespace"] = True
|
||||
safe_step = "Switch to the reviewer MCP session (e.g. gitea-reviewer) which has reviewer permissions configured, or ask the operator to update GITEA_MCP_PROFILE to a reviewer profile."
|
||||
safe_step = f"Switch to the {role_noun} MCP session (e.g. gitea-{role_noun}) which has {role_noun} permissions configured, or ask the operator to update GITEA_MCP_PROFILE to a {role_noun} profile."
|
||||
|
||||
result["safe_next_step"] = safe_step
|
||||
return result
|
||||
@@ -4264,6 +4276,14 @@ def gitea_merge_pr(
|
||||
_verify_role_mutation_workspace(
|
||||
remote, worktree_path=worktree_path, task="merge_pr"
|
||||
)
|
||||
allowed = get_profile()["allowed_operations"]
|
||||
forbidden = get_profile()["forbidden_operations"]
|
||||
active_role = _role_kind(allowed, forbidden)
|
||||
if active_role == "reviewer":
|
||||
raise RuntimeError(
|
||||
"Reviewer profile cannot merge. Use a merger profile/session "
|
||||
"after formal approval at current head."
|
||||
)
|
||||
workflow_blockers = _review_workflow_load_gate_reasons()
|
||||
do = (do or "").strip().lower()
|
||||
result = {
|
||||
@@ -4280,6 +4300,10 @@ def gitea_merge_pr(
|
||||
"merge_result": None,
|
||||
"merge_commit": None,
|
||||
"reasons": [],
|
||||
"active_profile": get_profile()["profile_name"],
|
||||
"role_kind": active_role,
|
||||
"merge_capability_source": "profile" if "gitea.pr.merge" in allowed else "config",
|
||||
"explicit_operator_auth": confirmation,
|
||||
}
|
||||
reasons = result["reasons"]
|
||||
if workflow_blockers:
|
||||
@@ -4449,7 +4473,7 @@ def gitea_merge_pr(
|
||||
# A profile/identity flip or side-channel override between preflight
|
||||
# and merge fails closed here.
|
||||
try:
|
||||
verify_mutation_authority(remote, host, required_role="reviewer",
|
||||
verify_mutation_authority(remote, host, required_role="merger",
|
||||
active_identity=auth_user)
|
||||
except RuntimeError as e:
|
||||
reasons.append(str(e))
|
||||
@@ -6786,26 +6810,28 @@ def _role_kind(allowed, forbidden) -> str:
|
||||
"""Classify the active profile from its normalized operations.
|
||||
|
||||
'reconciler' can close already-landed PRs without review/author powers;
|
||||
'author' can create PRs / push branches; 'reviewer' can approve/merge;
|
||||
'reconciler' can close already-landed PRs via ``gitea.pr.close`` without
|
||||
review/author powers; 'mixed' can do both (a config smell — the
|
||||
reviewer-deadlock invariant forbids it in v2 configs); 'limited' can do
|
||||
neither.
|
||||
'author' can create PRs / push branches; 'reviewer' can approve;
|
||||
'merger' can merge PRs;
|
||||
'mixed' can do multiple (a config smell); 'limited' can do neither.
|
||||
"""
|
||||
if reconciler_profile.is_reconciler_profile(allowed, forbidden):
|
||||
return "reconciler"
|
||||
def can(op):
|
||||
return gitea_config.check_operation(op, allowed, forbidden)[0]
|
||||
review = can("gitea.pr.approve") or can("gitea.pr.merge")
|
||||
can_merge = can("gitea.pr.merge")
|
||||
can_approve = can("gitea.pr.approve")
|
||||
author = can("gitea.pr.create") or can("gitea.branch.push")
|
||||
reconciler = (
|
||||
can("gitea.pr.close")
|
||||
and not review
|
||||
and not can_approve
|
||||
and not can_merge
|
||||
and not author
|
||||
)
|
||||
if review and author:
|
||||
if (can_approve or can_merge) and author:
|
||||
return "mixed"
|
||||
if review:
|
||||
if can_merge:
|
||||
return "merger"
|
||||
if can_approve:
|
||||
return "reviewer"
|
||||
if reconciler:
|
||||
return "reconciler"
|
||||
@@ -7323,12 +7349,20 @@ def mcp_get_control_plane_guide(
|
||||
"gitea.issue.comment (separate from gitea.pr.comment).")
|
||||
elif role == "reviewer":
|
||||
guidance.append(
|
||||
"Reviewer profile: review/approve/merge may proceed ONLY after "
|
||||
"Reviewer profile: review/approve may proceed ONLY after "
|
||||
"gitea_check_pr_eligibility passes and the PR head SHA is "
|
||||
"pinned (expected_head_sha); the PR author must be a different "
|
||||
"user, and merging additionally requires explicit operator "
|
||||
"authorization plus the 'MERGE PR <n>' confirmation. "
|
||||
"PR comments do not imply issue comments.")
|
||||
"user. Reviewer profiles cannot merge PRs. "
|
||||
"PR comments do not imply issue comments. "
|
||||
"Review and merge are separate workflow roles. A reviewer approval is not merge authorization.")
|
||||
elif role == "merger":
|
||||
guidance.append(
|
||||
"Merger profile: merge may proceed ONLY after formal approval at "
|
||||
"current head, gitea_check_pr_eligibility passes, and the PR head "
|
||||
"SHA is pinned (expected_head_sha); the PR author must be a different "
|
||||
"user, and merging requires explicit operator authorization plus the "
|
||||
"'MERGE PR <n>' confirmation. "
|
||||
"Review and merge are separate workflow roles. A reviewer approval is not merge authorization.")
|
||||
elif role == "mixed":
|
||||
guidance.append(
|
||||
"WARNING: this profile allows both authoring and "
|
||||
|
||||
Reference in New Issue
Block a user