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:
2026-07-13 21:48:28 -04:00
parent cc3ad0870a
commit 553f745f23
16 changed files with 1538 additions and 86 deletions
+384 -10
View File
@@ -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