feat: structured permission failure reports on gated tools (#142)
Every gated denial now carries a 'permission_report' explaining itself: requested/missing/required operation, active profile and identity (when already resolved), the profile's allowed operations, which configured profiles could perform the operation (names only), whether runtime switching is supported, whether a different MCP namespace/session is required, and the exact safe next action. New fail-soft helper _permission_block_report builds the report only after a gate has refused — it adds guidance to denials, never widens a permission, performs no network I/O, and degrades to a minimal fail-closed report if the profile itself cannot be resolved. Wired into gitea_check_pr_eligibility (and via it gitea_submit_pr_review, gitea_merge_pr, and the legacy gitea_review_pr wrapper) and into the issue-comment gate consumers gitea_list_issue_comments and gitea_create_issue_comment. Input errors (e.g. empty comment body) are not permission failures and get no report. Tests cover: missing gitea.issue.comment, author attempting review and merge, reviewer attempting an authoring operation, unknown/missing profile fail-closed, switching-enabled guidance, eligible calls carry no report, and no secret/URL material in any report. Closes #142 Co-Authored-By: Claude Fable 5 <[email protected]>
This commit is contained in:
Claude Fable 5
+117
-9
@@ -502,7 +502,9 @@ def gitea_check_pr_eligibility(
|
||||
repo: Override the repository name.
|
||||
|
||||
Returns:
|
||||
dict with 'eligible' (bool), the inputs inspected, and 'reasons'.
|
||||
dict with 'eligible' (bool), the inputs inspected, and 'reasons';
|
||||
when the block is a missing profile permission, also a structured
|
||||
'permission_report' (#142).
|
||||
"""
|
||||
action = (action or "").strip().lower()
|
||||
profile = get_profile()
|
||||
@@ -623,9 +625,16 @@ def gitea_check_pr_eligibility(
|
||||
# Determine missing permission
|
||||
missing_perm = None
|
||||
if not op_ok:
|
||||
missing_perm = f"gitea.pr.{action}" if action in ("approve", "request_changes", "merge") else f"gitea.{action}"
|
||||
missing_perm = (
|
||||
f"gitea.pr.{action}"
|
||||
if action in ("approve", "request_changes", "merge", "review")
|
||||
else f"gitea.{action}")
|
||||
|
||||
result["missing_permission"] = missing_perm
|
||||
if missing_perm:
|
||||
# Structured denial guidance (#142) — read-only, never widens.
|
||||
result["permission_report"] = _permission_block_report(
|
||||
missing_perm, identity=auth_user)
|
||||
|
||||
# Determine required profile
|
||||
req_profile = None
|
||||
@@ -830,6 +839,8 @@ def gitea_submit_pr_review(
|
||||
f"eligibility check for '{eligibility_action}' failed (fail closed)"
|
||||
)
|
||||
reasons.extend(elig.get("reasons", []))
|
||||
if elig.get("permission_report"):
|
||||
result["permission_report"] = elig["permission_report"]
|
||||
return result
|
||||
|
||||
# Gate 3 — redundant self-approval block (belt-and-suspenders over #14).
|
||||
@@ -1150,6 +1161,8 @@ def gitea_merge_pr(
|
||||
if not elig.get("eligible"):
|
||||
reasons.append("eligibility check for 'merge' failed (fail closed)")
|
||||
reasons.extend(elig.get("reasons", []))
|
||||
if elig.get("permission_report"):
|
||||
result["permission_report"] = elig["permission_report"]
|
||||
return result
|
||||
|
||||
# Gate 4 — head SHA must match if the caller pinned a reviewed SHA.
|
||||
@@ -1305,7 +1318,10 @@ def gitea_review_pr(
|
||||
return {"success": True, "message": f"Successfully submitted review for PR #{pr_number} with event '{event}'."}
|
||||
else:
|
||||
reasons = result.get("reasons", [])
|
||||
return {"success": False, "message": f"Review submission failed eligibility gates: {reasons}"}
|
||||
out = {"success": False, "message": f"Review submission failed eligibility gates: {reasons}"}
|
||||
if result.get("permission_report"):
|
||||
out["permission_report"] = result["permission_report"]
|
||||
return out
|
||||
|
||||
|
||||
@mcp.tool()
|
||||
@@ -1454,6 +1470,88 @@ def gitea_view_issue(
|
||||
return _with_optional_url(result, i.get("html_url"))
|
||||
|
||||
|
||||
def _permission_block_report(required_operation: str,
|
||||
identity: str | None = None) -> dict:
|
||||
"""Structured, LLM-safe explanation of a permission denial (#142).
|
||||
|
||||
Built only after a gate has already refused; it adds guidance to the
|
||||
refusal and never widens any permission, performs network I/O, or
|
||||
raises (fail-soft: degrades to a minimal fail-closed report). Names
|
||||
configured profiles only — never auth references, tokens, endpoint
|
||||
URLs, or keychain IDs.
|
||||
"""
|
||||
report = {
|
||||
"requested_operation": required_operation,
|
||||
"missing_permission": required_operation,
|
||||
"required_permission": required_operation,
|
||||
"active_profile": None,
|
||||
"active_identity": identity,
|
||||
"active_allowed_operations": [],
|
||||
"matching_configured_profiles": [],
|
||||
"runtime_switching_supported": False,
|
||||
"different_mcp_namespace_required": True,
|
||||
"exact_safe_next_action": (
|
||||
"Ask the operator to fix GITEA_MCP_CONFIG/GITEA_MCP_PROFILE; "
|
||||
"the active profile could not be resolved (fail closed)."),
|
||||
}
|
||||
try:
|
||||
profile = get_profile()
|
||||
except Exception:
|
||||
return report
|
||||
report["active_profile"] = profile.get("profile_name")
|
||||
if identity is None:
|
||||
report["active_identity"] = profile.get("identity")
|
||||
report["active_allowed_operations"] = (
|
||||
profile.get("allowed_operations") or [])
|
||||
|
||||
matching = []
|
||||
try:
|
||||
config = gitea_config.load_config() or {}
|
||||
for name, p in (config.get("profiles") or {}).items():
|
||||
p_allowed = []
|
||||
for op in p.get("allowed_operations") or []:
|
||||
try:
|
||||
p_allowed.append(gitea_config.normalize_operation(op))
|
||||
except Exception:
|
||||
pass
|
||||
p_forbidden = []
|
||||
for op in p.get("forbidden_operations") or []:
|
||||
try:
|
||||
p_forbidden.append(gitea_config.normalize_operation(op))
|
||||
except Exception:
|
||||
pass
|
||||
ok, _ = gitea_config.check_operation(
|
||||
required_operation, p_allowed, p_forbidden)
|
||||
if ok:
|
||||
matching.append(name)
|
||||
except Exception:
|
||||
matching = []
|
||||
report["matching_configured_profiles"] = matching
|
||||
|
||||
try:
|
||||
switching = gitea_config.is_runtime_switching_enabled()
|
||||
except Exception:
|
||||
switching = False
|
||||
report["runtime_switching_supported"] = switching
|
||||
|
||||
role = "reviewer" if required_operation in (
|
||||
"gitea.pr.approve", "gitea.pr.merge", "gitea.pr.request_changes",
|
||||
"gitea.pr.review") else "author"
|
||||
if switching and matching:
|
||||
report["different_mcp_namespace_required"] = False
|
||||
report["exact_safe_next_action"] = (
|
||||
f"Call gitea_activate_profile with a profile that allows "
|
||||
f"{required_operation} (matching configured profiles: "
|
||||
f"{matching}).")
|
||||
else:
|
||||
report["different_mcp_namespace_required"] = True
|
||||
report["exact_safe_next_action"] = (
|
||||
f"Switch to the {role} MCP session (e.g. gitea-{role}), or ask "
|
||||
f"the operator to set GITEA_MCP_PROFILE to a profile that "
|
||||
f"allows {required_operation}.")
|
||||
return report
|
||||
|
||||
|
||||
def _issue_comment_gate(op: str) -> list[str]:
|
||||
"""Profile permission check for issue-comment tools (#126).
|
||||
|
||||
@@ -1509,12 +1607,14 @@ def gitea_list_issue_comments(
|
||||
Returns:
|
||||
dict with 'success', 'issue_number', and 'comments' (each with 'id',
|
||||
'author', 'created_at', 'updated_at', 'body'); on a permission block,
|
||||
'success' False and 'reasons' with no API call made.
|
||||
'success' False, 'reasons', and a structured 'permission_report'
|
||||
(#142) with no API call made.
|
||||
"""
|
||||
reasons = _issue_comment_gate("gitea.read")
|
||||
if reasons:
|
||||
return {"success": False, "issue_number": issue_number,
|
||||
"reasons": reasons}
|
||||
"reasons": reasons,
|
||||
"permission_report": _permission_block_report("gitea.read")}
|
||||
h, o, r = _resolve(remote, host, org, repo)
|
||||
auth = _auth(h)
|
||||
api = f"{repo_api_url(h, o, r)}/issues/{issue_number}/comments"
|
||||
@@ -1568,14 +1668,22 @@ def gitea_create_issue_comment(
|
||||
Returns:
|
||||
dict with 'success', 'comment_id', and 'issue_number' ('url' only
|
||||
with the reveal opt-in); on a permission block or empty body,
|
||||
'success'/'performed' False and 'reasons' with no API call made.
|
||||
'success'/'performed' False and 'reasons' with no API call made
|
||||
(permission blocks also carry a structured 'permission_report',
|
||||
#142).
|
||||
"""
|
||||
reasons = _issue_comment_gate("gitea.issue.comment")
|
||||
gate_reasons = _issue_comment_gate("gitea.issue.comment")
|
||||
reasons = list(gate_reasons)
|
||||
if not (body or "").strip():
|
||||
reasons.append("comment body must be a non-empty string")
|
||||
if reasons:
|
||||
return {"success": False, "performed": False,
|
||||
"issue_number": issue_number, "reasons": reasons}
|
||||
blocked = {"success": False, "performed": False,
|
||||
"issue_number": issue_number, "reasons": reasons}
|
||||
if gate_reasons:
|
||||
# Permission denial (not a mere input error): explain it (#142).
|
||||
blocked["permission_report"] = _permission_block_report(
|
||||
"gitea.issue.comment")
|
||||
return blocked
|
||||
h, o, r = _resolve(remote, host, org, repo)
|
||||
auth = _auth(h)
|
||||
api = f"{repo_api_url(h, o, r)}/issues/{issue_number}/comments"
|
||||
|
||||
Reference in New Issue
Block a user