feat(guard): native MCP transport binding and contaminated-review quarantine (Closes #695)
Bind mutation/credential paths to a process-local native MCP runtime so env spoofing, direct imports, and offline helpers cannot reconstruct session gates after native transport failure. Quarantine contaminated formal reviews under controller authority and honor quarantine in review feedback, merge eligibility, merge mutation, and canonical handoff validation. Add regression coverage for the second (PR #694 / review 427) incident class.
This commit is contained in:
+384
-10
@@ -1109,6 +1109,8 @@ import issue_lock_store # noqa: E402
|
||||
import issue_lock_adoption # noqa: E402
|
||||
import stacked_pr_support # noqa: E402
|
||||
import merge_approval_gate # noqa: E402
|
||||
import review_quarantine # noqa: E402 # #695 contaminated formal-review quarantine
|
||||
import mcp_daemon_guard # noqa: E402 # #695 native transport provenance
|
||||
import already_landed_reconcile # noqa: E402
|
||||
import author_mutation_worktree # noqa: E402
|
||||
import root_checkout_guard # noqa: E402
|
||||
@@ -3055,6 +3057,51 @@ def gitea_check_pr_eligibility(
|
||||
elif result["mergeable"] is None:
|
||||
reasons.append("PR mergeability unknown")
|
||||
|
||||
# #695: merge eligibility must honor quarantine-aware formal review feedback.
|
||||
# Contaminated approvals (e.g. review 427 on PR #694) must not make merge
|
||||
# eligible even when Gitea still shows APPROVED / mergeable=true.
|
||||
if action == "merge" and not reasons:
|
||||
try:
|
||||
feedback = gitea_get_pr_review_feedback(
|
||||
pr_number=pr_number, remote=remote, host=host, org=org, repo=repo,
|
||||
)
|
||||
except Exception as exc: # noqa: BLE001 — fail closed, never leak secrets
|
||||
feedback = {
|
||||
"success": False,
|
||||
"reasons": [
|
||||
"PR review feedback unavailable for merge eligibility "
|
||||
f"(fail closed, #695): {_redact(str(exc))}"
|
||||
],
|
||||
}
|
||||
result["approval_visible"] = feedback.get("approval_visible")
|
||||
result["approval_at_current_head"] = feedback.get("approval_at_current_head")
|
||||
result["quarantined_approvals_at_current_head"] = feedback.get(
|
||||
"quarantined_approvals_at_current_head"
|
||||
)
|
||||
result["stale_approval_block_reason"] = feedback.get(
|
||||
"stale_approval_block_reason"
|
||||
)
|
||||
result["has_blocking_change_requests"] = feedback.get(
|
||||
"has_blocking_change_requests"
|
||||
)
|
||||
if not feedback.get("success"):
|
||||
reasons.append(
|
||||
"PR review feedback unavailable for merge eligibility (fail closed, #695)"
|
||||
)
|
||||
reasons.extend(feedback.get("reasons") or [])
|
||||
elif feedback.get("has_blocking_change_requests"):
|
||||
reasons.append(
|
||||
"undismissed REQUEST_CHANGES review blocks merge eligibility (fail closed)"
|
||||
)
|
||||
elif not feedback.get("approval_at_current_head"):
|
||||
reasons.append(
|
||||
feedback.get("stale_approval_block_reason")
|
||||
or (
|
||||
"no non-quarantined APPROVED review at current head; "
|
||||
"merge eligibility denied (#695)"
|
||||
)
|
||||
)
|
||||
|
||||
result["eligible"] = len(reasons) == 0
|
||||
if result["eligible"]:
|
||||
reasons.append("all eligibility checks passed")
|
||||
@@ -3453,6 +3500,10 @@ def record_live_review_mutation(pr_number: int, action: str, review_id: int | No
|
||||
"action": action,
|
||||
"review_id": review_id,
|
||||
"review_state": action,
|
||||
# #695 AC6: audit records expose native session/transport provenance.
|
||||
**mcp_daemon_guard.mutation_provenance_fields(),
|
||||
"writer_pid": os.getpid(),
|
||||
"session_pid": lock.get("session_pid") or os.getpid(),
|
||||
}
|
||||
if head_sha:
|
||||
entry["head_sha"] = head_sha
|
||||
@@ -3678,30 +3729,68 @@ def gitea_get_pr_review_feedback(
|
||||
base = f"{repo_api_url(h, o, r)}/pulls/{pr_number}"
|
||||
pr = api_request("GET", base, auth) or {}
|
||||
raw_reviews = api_request("GET", f"{base}/reviews", auth) or []
|
||||
if not isinstance(pr, dict):
|
||||
return {
|
||||
"success": False,
|
||||
"pr_number": pr_number,
|
||||
"feedback_not_attempted": True,
|
||||
"reasons": ["PR payload unavailable for review feedback (fail closed)"],
|
||||
}
|
||||
if not isinstance(raw_reviews, list):
|
||||
raw_reviews = []
|
||||
current_head = (pr.get("head") or {}).get("sha")
|
||||
reveal = _reveal_endpoints()
|
||||
|
||||
ordered = sorted(
|
||||
raw_reviews,
|
||||
(rv for rv in raw_reviews if isinstance(rv, dict)),
|
||||
key=lambda rv: ((rv.get("submitted_at") or ""), rv.get("id") or 0),
|
||||
)
|
||||
reviews = []
|
||||
latest_by_reviewer = {}
|
||||
latest_reviewed_head = None
|
||||
quarantined_review_ids: set[int] = set()
|
||||
quarantined_at_head = 0
|
||||
for rv in ordered:
|
||||
state = (rv.get("state") or "").upper()
|
||||
reviewer = (rv.get("user") or {}).get("login", "")
|
||||
commit_id = rv.get("commit_id")
|
||||
rid = rv.get("id")
|
||||
try:
|
||||
rid_int = int(rid) if rid is not None else None
|
||||
except (TypeError, ValueError):
|
||||
rid_int = None
|
||||
q = review_quarantine.is_review_quarantined(
|
||||
remote=remote,
|
||||
org=o,
|
||||
repo=r,
|
||||
pr_number=pr_number,
|
||||
review_id=rid_int,
|
||||
reviewed_head_sha=commit_id or current_head,
|
||||
)
|
||||
entry = {
|
||||
"reviewer": reviewer,
|
||||
"verdict": state,
|
||||
"body": _redact(rv.get("body") or ""),
|
||||
"submitted_at": rv.get("submitted_at"),
|
||||
"reviewed_head_sha": commit_id,
|
||||
"review_id": rid_int,
|
||||
"dismissed": bool(rv.get("dismissed")),
|
||||
"stale": bool(rv.get("stale")) or bool(
|
||||
commit_id and current_head and commit_id != current_head),
|
||||
"quarantined": bool(q.get("quarantined")),
|
||||
}
|
||||
if q.get("quarantined"):
|
||||
entry["quarantine_reasons"] = q.get("reasons") or []
|
||||
if rid_int is not None:
|
||||
quarantined_review_ids.add(rid_int)
|
||||
if (
|
||||
state == "APPROVED"
|
||||
and not entry["dismissed"]
|
||||
and current_head
|
||||
and commit_id
|
||||
and commit_id == current_head
|
||||
):
|
||||
quarantined_at_head += 1
|
||||
if reveal:
|
||||
entry["url"] = rv.get("html_url")
|
||||
reviews.append(entry)
|
||||
@@ -3709,7 +3798,11 @@ def gitea_get_pr_review_feedback(
|
||||
# per-reviewer verdict — otherwise a drive-by comment on the
|
||||
# current head would mask the staleness of an older undismissed
|
||||
# REQUEST_CHANGES.
|
||||
# #695: quarantined formal reviews never authorize merge and must not
|
||||
# become the reviewer's latest active verdict for eligibility/merge.
|
||||
if state in _VERDICT_STATES and state != "COMMENT":
|
||||
if entry.get("quarantined"):
|
||||
continue
|
||||
latest_reviewed_head = commit_id or latest_reviewed_head
|
||||
if reviewer:
|
||||
latest_by_reviewer[reviewer] = entry
|
||||
@@ -3725,7 +3818,21 @@ def gitea_get_pr_review_feedback(
|
||||
approval_head = merge_approval_gate.assess_merge_approval_head(
|
||||
current_head_sha=current_head,
|
||||
latest_by_reviewer=latest_by_reviewer,
|
||||
quarantined_review_ids=quarantined_review_ids,
|
||||
)
|
||||
stale_reason = approval_head["stale_approval_block_reason"]
|
||||
# When the only approvals at head are quarantined, they were excluded from
|
||||
# latest_by_reviewer; surface an explicit #695 void reason for eligibility/merge.
|
||||
if (
|
||||
not approval_head["approval_at_current_head"]
|
||||
and quarantined_at_head
|
||||
and not stale_reason
|
||||
):
|
||||
stale_reason = (
|
||||
"contaminated/quarantined approval at current head is void for "
|
||||
"merge authorization (#695); required next action: fresh native "
|
||||
"MCP re-review after controller quarantine evidence is recorded"
|
||||
)
|
||||
return {
|
||||
"success": True,
|
||||
"pr_number": pr_number,
|
||||
@@ -3738,7 +3845,14 @@ def gitea_get_pr_review_feedback(
|
||||
"approval_visible": bool(approvals),
|
||||
"approval_at_current_head": approval_head["approval_at_current_head"],
|
||||
"latest_approved_head_sha": approval_head["latest_approved_head_sha"],
|
||||
"stale_approval_block_reason": approval_head["stale_approval_block_reason"],
|
||||
"stale_approval_block_reason": stale_reason,
|
||||
"quarantined_review_ids": sorted(quarantined_review_ids),
|
||||
"quarantined_approvals_at_current_head": (
|
||||
max(
|
||||
approval_head.get("quarantined_approvals_at_current_head") or 0,
|
||||
quarantined_at_head,
|
||||
)
|
||||
),
|
||||
"latest_reviewed_head_sha": latest_reviewed_head,
|
||||
"review_feedback_stale": bool(
|
||||
latest_reviewed_head and current_head
|
||||
@@ -3747,6 +3861,7 @@ def gitea_get_pr_review_feedback(
|
||||
e["reviewed_head_sha"] and current_head
|
||||
and e["reviewed_head_sha"] != current_head
|
||||
for e in blocking),
|
||||
"native_runtime": mcp_daemon_guard.native_runtime_status(),
|
||||
}
|
||||
|
||||
|
||||
@@ -5414,6 +5529,16 @@ def gitea_merge_pr(
|
||||
result["pr_author"] = elig.get("pr_author")
|
||||
result["head_sha"] = elig.get("head_sha")
|
||||
result["mergeable"] = elig.get("mergeable")
|
||||
# Surface #695 approval/quarantine fields from eligibility even on deny.
|
||||
for _k in (
|
||||
"approval_visible",
|
||||
"approval_at_current_head",
|
||||
"quarantined_approvals_at_current_head",
|
||||
"stale_approval_block_reason",
|
||||
"has_blocking_change_requests",
|
||||
):
|
||||
if _k in elig:
|
||||
result[_k] = elig.get(_k)
|
||||
if not elig.get("eligible"):
|
||||
reasons.append("eligibility check for 'merge' failed (fail closed)")
|
||||
reasons.extend(elig.get("reasons", []))
|
||||
@@ -5521,16 +5646,31 @@ def gitea_merge_pr(
|
||||
result["review_feedback_stale"] = feedback.get("review_feedback_stale")
|
||||
result["has_blocking_change_requests"] = feedback.get(
|
||||
"has_blocking_change_requests")
|
||||
result["quarantined_review_ids"] = feedback.get("quarantined_review_ids") or []
|
||||
result["quarantined_approvals_at_current_head"] = feedback.get(
|
||||
"quarantined_approvals_at_current_head"
|
||||
)
|
||||
if feedback.get("has_blocking_change_requests"):
|
||||
reasons.append(
|
||||
"undismissed REQUEST_CHANGES review blocks merge (fail closed)"
|
||||
)
|
||||
return result
|
||||
if not feedback.get("approval_visible"):
|
||||
reasons.append(
|
||||
"no visible APPROVED review on PR; verify review submission "
|
||||
"completed before merge (fail closed)"
|
||||
)
|
||||
# #695: quarantined APPROVED reviews are not "visible" for merge auth.
|
||||
if feedback.get("quarantined_approvals_at_current_head"):
|
||||
reasons.append(
|
||||
feedback.get("stale_approval_block_reason")
|
||||
or (
|
||||
"only contaminated/quarantined approval(s) at current head "
|
||||
"(#695); merge authorization is void — fresh native MCP "
|
||||
"re-review required after controller quarantine evidence"
|
||||
)
|
||||
)
|
||||
else:
|
||||
reasons.append(
|
||||
"no visible APPROVED review on PR; verify review submission "
|
||||
"completed before merge (fail closed)"
|
||||
)
|
||||
return result
|
||||
if not feedback.get("approval_at_current_head"):
|
||||
reasons.append(
|
||||
@@ -12673,13 +12813,247 @@ def gitea_reclaim_expired_workflow_lease(
|
||||
}
|
||||
|
||||
|
||||
@mcp.tool()
|
||||
def gitea_quarantine_contaminated_review(
|
||||
pr_number: int,
|
||||
review_id: int,
|
||||
confirmation: str,
|
||||
reason: str,
|
||||
reviewed_head_sha: str,
|
||||
incident_issue: int = 695,
|
||||
forensic_comment_ids: list[int] | None = None,
|
||||
remote: str = "dadeschools",
|
||||
host: str | None = None,
|
||||
org: str | None = None,
|
||||
repo: str | None = None,
|
||||
post_audit_comment: bool = True,
|
||||
) -> dict:
|
||||
"""Controller quarantine of a contaminated formal review (#695 AC8).
|
||||
|
||||
Writes a durable quarantine record that live feedback / eligibility /
|
||||
merge gates honor by ``review_id``. Forensic Gitea review objects and
|
||||
historical comments are retained (never deleted).
|
||||
|
||||
**Native transport only.** Untrusted local imports / offline scripts
|
||||
cannot create quarantine records. Confirmation must equal exactly
|
||||
``QUARANTINE CONTAMINATED REVIEW <review_id> PR <pr_number>``.
|
||||
|
||||
Restricted to reconciler / merger / controller profiles (not author or
|
||||
the contaminated reviewer acting alone). Does not auto-apply to review
|
||||
427 until an independent adversarial reviewer and controller deployment
|
||||
of this tooling have completed.
|
||||
"""
|
||||
# Fail closed outside native MCP before any mutation assessment.
|
||||
try:
|
||||
mcp_daemon_guard.assert_sanctioned_mutation_runtime(
|
||||
"gitea_quarantine_contaminated_review"
|
||||
)
|
||||
except mcp_daemon_guard.UnsanctionedRuntimeError as exc:
|
||||
return {
|
||||
"success": False,
|
||||
"quarantined": False,
|
||||
"reasons": [str(exc)],
|
||||
"native_runtime": mcp_daemon_guard.native_runtime_status(),
|
||||
}
|
||||
|
||||
assessment = review_quarantine.assess_quarantine_write(
|
||||
confirmation=confirmation,
|
||||
pr_number=pr_number,
|
||||
review_id=review_id,
|
||||
reason=reason,
|
||||
native_required=True,
|
||||
)
|
||||
if not assessment.get("allowed"):
|
||||
return {
|
||||
"success": False,
|
||||
"quarantined": False,
|
||||
"reasons": assessment.get("reasons") or [],
|
||||
"expected_confirmation": assessment.get("expected_confirmation"),
|
||||
"native_runtime": mcp_daemon_guard.native_runtime_status(),
|
||||
}
|
||||
|
||||
profile = get_profile()
|
||||
profile_name = (profile.get("profile_name") or "").strip()
|
||||
role = _actual_profile_role()
|
||||
allowed_roles = {"reconciler", "merger"}
|
||||
controller_named = "controller" in profile_name.lower()
|
||||
if role not in allowed_roles and not controller_named:
|
||||
return {
|
||||
"success": False,
|
||||
"quarantined": False,
|
||||
"reasons": [
|
||||
f"quarantine requires reconciler/merger/controller profile "
|
||||
f"(active role={role!r}, profile={profile_name!r}); "
|
||||
"author/reviewer-only sessions cannot quarantine (#695)"
|
||||
],
|
||||
"native_runtime": mcp_daemon_guard.native_runtime_status(),
|
||||
}
|
||||
|
||||
comment_block = _profile_operation_gate("gitea.pr.comment")
|
||||
if comment_block and post_audit_comment:
|
||||
return {
|
||||
"success": False,
|
||||
"quarantined": False,
|
||||
"reasons": comment_block,
|
||||
"permission_report": _permission_block_report("gitea.pr.comment"),
|
||||
}
|
||||
read_block = _profile_operation_gate("gitea.read")
|
||||
if read_block:
|
||||
return {
|
||||
"success": False,
|
||||
"quarantined": False,
|
||||
"reasons": read_block,
|
||||
"permission_report": _permission_block_report("gitea.read"),
|
||||
}
|
||||
|
||||
h, o, r = _resolve(remote, host, org, repo)
|
||||
auth = _auth(h)
|
||||
try:
|
||||
identity = _authenticated_username(h) or profile.get("username") or ""
|
||||
except Exception:
|
||||
identity = profile.get("username") or ""
|
||||
|
||||
# Verify review exists (forensic retention — do not dismiss via Gitea API).
|
||||
try:
|
||||
reviews = (
|
||||
api_request(
|
||||
"GET",
|
||||
f"{repo_api_url(h, o, r)}/pulls/{pr_number}/reviews",
|
||||
auth,
|
||||
)
|
||||
or []
|
||||
)
|
||||
except Exception as exc: # noqa: BLE001
|
||||
return {
|
||||
"success": False,
|
||||
"quarantined": False,
|
||||
"reasons": [
|
||||
f"could not list PR reviews before quarantine (fail closed): "
|
||||
f"{_redact(str(exc))}"
|
||||
],
|
||||
}
|
||||
match = None
|
||||
for rv in reviews:
|
||||
if int(rv.get("id") or 0) == int(review_id):
|
||||
match = rv
|
||||
break
|
||||
if match is None:
|
||||
return {
|
||||
"success": False,
|
||||
"quarantined": False,
|
||||
"reasons": [
|
||||
f"review_id {review_id} not found on PR #{pr_number} "
|
||||
f"(fail closed; refuse quarantine of missing review)"
|
||||
],
|
||||
}
|
||||
live_head = (match.get("commit_id") or "").strip().lower()
|
||||
want_head = (reviewed_head_sha or "").strip().lower()
|
||||
if want_head and live_head and want_head != live_head:
|
||||
return {
|
||||
"success": False,
|
||||
"quarantined": False,
|
||||
"reasons": [
|
||||
f"reviewed_head_sha {want_head[:12]}… does not match review "
|
||||
f"commit_id {live_head[:12]}… (fail closed, #695)"
|
||||
],
|
||||
}
|
||||
|
||||
record = review_quarantine.build_quarantine_record(
|
||||
remote=remote,
|
||||
org=o,
|
||||
repo=r,
|
||||
pr_number=pr_number,
|
||||
review_id=review_id,
|
||||
reviewed_head_sha=want_head or live_head,
|
||||
reason=reason,
|
||||
actor_username=identity,
|
||||
profile_name=profile_name,
|
||||
incident_issue=incident_issue,
|
||||
forensic_comment_ids=forensic_comment_ids,
|
||||
)
|
||||
try:
|
||||
written = review_quarantine.write_quarantine_record(record)
|
||||
except mcp_daemon_guard.UnsanctionedRuntimeError as exc:
|
||||
return {
|
||||
"success": False,
|
||||
"quarantined": False,
|
||||
"reasons": [str(exc)],
|
||||
"native_runtime": mcp_daemon_guard.native_runtime_status(),
|
||||
}
|
||||
except OSError as exc:
|
||||
return {
|
||||
"success": False,
|
||||
"quarantined": False,
|
||||
"reasons": [f"quarantine persist failed: {_redact(str(exc))}"],
|
||||
}
|
||||
|
||||
audit_comment_id = None
|
||||
if post_audit_comment:
|
||||
body = review_quarantine.format_quarantine_audit_comment(record)
|
||||
try:
|
||||
with _audited(
|
||||
"comment_pr",
|
||||
host=h,
|
||||
remote=remote,
|
||||
org=o,
|
||||
repo=r,
|
||||
pr_number=pr_number,
|
||||
request_metadata={"source": "quarantine_contaminated_review"},
|
||||
):
|
||||
posted = api_request(
|
||||
"POST",
|
||||
f"{repo_api_url(h, o, r)}/issues/{pr_number}/comments",
|
||||
auth,
|
||||
{"body": body},
|
||||
)
|
||||
if isinstance(posted, dict):
|
||||
audit_comment_id = posted.get("id")
|
||||
except Exception as exc: # noqa: BLE001
|
||||
return {
|
||||
"success": True,
|
||||
"quarantined": True,
|
||||
"review_id": review_id,
|
||||
"pr_number": pr_number,
|
||||
"path": written.get("path"),
|
||||
"audit_comment_id": None,
|
||||
"warnings": [
|
||||
f"quarantine written but audit comment failed: "
|
||||
f"{_redact(str(exc))}"
|
||||
],
|
||||
"record": {
|
||||
k: v
|
||||
for k, v in record.items()
|
||||
if k != "native_provenance"
|
||||
} | {
|
||||
"native_provenance": record.get("native_provenance"),
|
||||
},
|
||||
"native_runtime": mcp_daemon_guard.native_runtime_status(),
|
||||
}
|
||||
|
||||
return {
|
||||
"success": True,
|
||||
"quarantined": True,
|
||||
"review_id": review_id,
|
||||
"pr_number": pr_number,
|
||||
"path": written.get("path"),
|
||||
"audit_comment_id": audit_comment_id,
|
||||
"retain_forensic_evidence": True,
|
||||
"merge_authorization": "void",
|
||||
"record": record,
|
||||
"native_runtime": mcp_daemon_guard.native_runtime_status(),
|
||||
"reasons": [
|
||||
f"review_id {review_id} quarantined for PR #{pr_number}; "
|
||||
"merge authorization void; forensic evidence retained (#695)"
|
||||
],
|
||||
}
|
||||
|
||||
|
||||
# ── Entry point ───────────────────────────────────────────────────────────────
|
||||
|
||||
if __name__ == "__main__":
|
||||
# #558: mark this process as the official MCP daemon before any tool
|
||||
# dispatch so direct shell imports cannot reuse mutation/auth paths.
|
||||
import mcp_daemon_guard
|
||||
|
||||
# #558 / #695: mark this process as the official native MCP daemon before
|
||||
# any tool dispatch. Env vars alone cannot reconstruct native transport;
|
||||
# offline imports / standalone scripts fail closed on mutations.
|
||||
mcp_daemon_guard.mark_sanctioned_daemon()
|
||||
# Lock this session's launch profile into the environment so child CLI
|
||||
# processes (e.g. review_pr.py) can detect and refuse profile
|
||||
|
||||
Reference in New Issue
Block a user