sysadmin
553f745f23
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.
429 lines
13 KiB
Python
429 lines
13 KiB
Python
"""Fail-closed validation for workflow-changing Gitea comments (#496)."""
|
|
|
|
from __future__ import annotations
|
|
|
|
import re
|
|
from typing import Any
|
|
|
|
VALID_ROLES = frozenset({
|
|
"controller",
|
|
"author",
|
|
"reviewer",
|
|
"merger",
|
|
"reconciler",
|
|
"user",
|
|
})
|
|
|
|
_BASE_REQUIRED = ("STATE", "WHO_IS_NEXT", "NEXT_ACTION", "NEXT_PROMPT", "WHY")
|
|
|
|
_ISSUE_REQUIRED = _BASE_REQUIRED + ("BLOCKERS", "VALIDATION")
|
|
_PR_REQUIRED = _BASE_REQUIRED + (
|
|
"ISSUE",
|
|
"HEAD_SHA",
|
|
"REVIEW_STATUS",
|
|
"MERGE_READY",
|
|
"BLOCKERS",
|
|
"VALIDATION",
|
|
)
|
|
_SUPERSESSION_REQUIRED = _BASE_REQUIRED + (
|
|
"CANONICAL_ITEM",
|
|
"SUPERSEDED_ITEM",
|
|
"CLOSE_OR_KEEP_OPEN",
|
|
)
|
|
|
|
_MACHINE_MARKERS = (
|
|
"<!-- mcp-review-lease:v1 -->",
|
|
"<!-- mcp-conflict-fix-lease:v1 -->",
|
|
"<!-- gitea-issue-claim-heartbeat:v1 -->",
|
|
)
|
|
|
|
_WORKFLOW_TRIGGERS = re.compile(
|
|
r"\b(?:"
|
|
r"blocked|unblocked|ready(?:\s+for\s+(?:review|merge|author))?|"
|
|
r"ready-to-merge|approved|approve|request\s+changes|changes\s+requested|"
|
|
r"superseded|duplicate|canonical|next\s+action|next\s+actor|who\s+is\s+next|"
|
|
r"author\s+should|reviewer\s+should|merger\s+should|reconciler\s+should|"
|
|
r"controller\s+should|issue\s+complete|pr\s+open|pr\s+merged|close\s+this|"
|
|
r"do\s+not\s+merge|needs?\s+rebase|stale\s+approval|contaminated\s+review|"
|
|
r"merge\s+ready|ready\s+for\s+merge"
|
|
r")\b",
|
|
re.IGNORECASE,
|
|
)
|
|
|
|
_CANONICAL_HEADINGS = (
|
|
"## Canonical Issue State",
|
|
"## Canonical PR State",
|
|
"## Canonical Discussion Summary",
|
|
)
|
|
|
|
_FIELD_RE = re.compile(
|
|
r"^([A-Z][A-Z0-9_]*)\s*:\s*(.*)$",
|
|
re.MULTILINE,
|
|
)
|
|
|
|
_VAGUE_NEXT_ACTIONS = frozenset({
|
|
"continue",
|
|
"handle this",
|
|
"fix it",
|
|
"follow up",
|
|
"follow-up",
|
|
"see above",
|
|
"see review",
|
|
"tbd",
|
|
"todo",
|
|
"as needed",
|
|
"proceed",
|
|
"next steps",
|
|
"will check",
|
|
"investigate",
|
|
})
|
|
|
|
_FULL_SHA_RE = re.compile(r"\b[0-9a-f]{40}\b", re.IGNORECASE)
|
|
_SHORT_SHA_RE = re.compile(r"\b[0-9a-f]{7,40}\b", re.IGNORECASE)
|
|
_PR_REF_RE = re.compile(r"(?:PR\s*#|pull\s*#)\d+|\b#\d{2,}\b", re.IGNORECASE)
|
|
_APPROVAL_PROOF_RE = re.compile(
|
|
r"\b(?:approved|approval_at_current_head|APPROVE)\b",
|
|
re.IGNORECASE,
|
|
)
|
|
_UNBLOCK_RE = re.compile(
|
|
r"\b(?:unblock|until|after|once|when|requires?|must)\b",
|
|
re.IGNORECASE,
|
|
)
|
|
|
|
|
|
def _parse_fields(body: str) -> dict[str, str]:
|
|
"""Parse KEY: value fields, including values continued on following lines."""
|
|
fields: dict[str, str] = {}
|
|
current_key: str | None = None
|
|
current_lines: list[str] = []
|
|
|
|
def _flush() -> None:
|
|
nonlocal current_key, current_lines
|
|
if current_key is not None:
|
|
fields[current_key] = "\n".join(current_lines).strip()
|
|
current_key = None
|
|
current_lines = []
|
|
|
|
for line in (body or "").splitlines():
|
|
if line.startswith("## "):
|
|
_flush()
|
|
continue
|
|
match = re.match(r"^([A-Z][A-Z0-9_]*)\s*:\s*(.*)$", line)
|
|
if match:
|
|
_flush()
|
|
current_key = match.group(1).strip().upper()
|
|
rest = match.group(2)
|
|
current_lines = [rest] if rest else []
|
|
elif current_key is not None:
|
|
current_lines.append(line)
|
|
_flush()
|
|
return fields
|
|
|
|
|
|
def _is_machine_generated(body: str) -> bool:
|
|
text = body or ""
|
|
return any(marker in text for marker in _MACHINE_MARKERS)
|
|
|
|
|
|
def _has_canonical_heading(body: str) -> bool:
|
|
return any(h in (body or "") for h in _CANONICAL_HEADINGS)
|
|
|
|
|
|
def is_workflow_changing_comment(body: str) -> bool:
|
|
"""True when comment text implies a workflow/state transition."""
|
|
text = (body or "").strip()
|
|
if not text:
|
|
return False
|
|
if _is_machine_generated(text):
|
|
return False
|
|
if _has_canonical_heading(text):
|
|
return True
|
|
if _WORKFLOW_TRIGGERS.search(text):
|
|
return True
|
|
fields = _parse_fields(text)
|
|
if "STATE" in fields or "WHO_IS_NEXT" in fields:
|
|
return True
|
|
return False
|
|
|
|
|
|
def infer_comment_context(body: str, *, explicit: str | None = None) -> str:
|
|
if explicit:
|
|
return explicit
|
|
text = body or ""
|
|
if "## Canonical Discussion Summary" in text:
|
|
return "discussion_summary"
|
|
if "## Canonical PR State" in text:
|
|
return "pr_comment"
|
|
if "## Canonical Issue State" in text:
|
|
return "issue_comment"
|
|
fields = _parse_fields(text)
|
|
if fields.get("CANONICAL_ITEM") or fields.get("SUPERSEDED_ITEM"):
|
|
return "supersession"
|
|
if any(k in fields for k in ("HEAD_SHA", "REVIEW_STATUS", "MERGE_READY", "ISSUE")):
|
|
return "pr_comment"
|
|
return "issue_comment"
|
|
|
|
|
|
def _required_fields_for_context(context: str) -> tuple[str, ...]:
|
|
if context in ("pr_comment", "pr_review"):
|
|
return _PR_REQUIRED
|
|
if context == "supersession":
|
|
return _SUPERSESSION_REQUIRED
|
|
if context == "discussion_summary":
|
|
return _BASE_REQUIRED + ("DECISION", "SUBSTANTIVE_COMMENTS")
|
|
return _ISSUE_REQUIRED
|
|
|
|
|
|
def _is_vague_next_action(value: str) -> bool:
|
|
normalized = re.sub(r"\s+", " ", (value or "").strip().lower())
|
|
normalized = normalized.rstrip(".")
|
|
if not normalized:
|
|
return True
|
|
if normalized in _VAGUE_NEXT_ACTIONS:
|
|
return True
|
|
if len(normalized) < 12:
|
|
return True
|
|
return False
|
|
|
|
|
|
def _next_prompt_ok(value: str) -> bool:
|
|
text = (value or "").strip()
|
|
if len(text) < 40:
|
|
return False
|
|
if text.lower() in {"n/a", "none", "tbd", "todo"}:
|
|
return False
|
|
return True
|
|
|
|
|
|
def _state_value(fields: dict[str, str]) -> str:
|
|
return (fields.get("STATE") or "").strip().lower()
|
|
|
|
|
|
def _suggested_template(context: str) -> str:
|
|
if context == "pr_comment" or context == "pr_review":
|
|
return (
|
|
"## Canonical PR State\n\n"
|
|
"STATE:\n"
|
|
"WHO_IS_NEXT:\n"
|
|
"NEXT_ACTION:\n"
|
|
"NEXT_PROMPT:\n"
|
|
"```text\n<paste-ready prompt>\n```\n"
|
|
"WHAT_HAPPENED:\n"
|
|
"WHY:\n"
|
|
"ISSUE:\n"
|
|
"HEAD_SHA:\n"
|
|
"REVIEW_STATUS:\n"
|
|
"MERGE_READY:\n"
|
|
"BLOCKERS:\n"
|
|
"VALIDATION:\n"
|
|
"LAST_UPDATED_BY:\n"
|
|
)
|
|
if context == "supersession":
|
|
return (
|
|
"## Canonical PR State\n\n"
|
|
"STATE:\nsuperseded\n"
|
|
"WHO_IS_NEXT:\nreconciler\n"
|
|
"NEXT_ACTION:\n"
|
|
"NEXT_PROMPT:\n"
|
|
"WHY:\n"
|
|
"CANONICAL_ITEM:\n"
|
|
"SUPERSEDED_ITEM:\n"
|
|
"CLOSE_OR_KEEP_OPEN:\n"
|
|
)
|
|
if context == "discussion_summary":
|
|
return (
|
|
"## Canonical Discussion Summary\n\n"
|
|
"STATE:\n"
|
|
"WHO_IS_NEXT:\n"
|
|
"DECISION:\n"
|
|
"WHY:\n"
|
|
"SUBSTANTIVE_COMMENTS:\n"
|
|
"NEXT_ACTION:\n"
|
|
"NEXT_PROMPT:\n"
|
|
)
|
|
return (
|
|
"## Canonical Issue State\n\n"
|
|
"STATE:\n"
|
|
"WHO_IS_NEXT:\n"
|
|
"NEXT_ACTION:\n"
|
|
"NEXT_PROMPT:\n"
|
|
"```text\n<paste-ready prompt>\n```\n"
|
|
"WHAT_HAPPENED:\n"
|
|
"WHY:\n"
|
|
"RELATED_PRS:\n"
|
|
"BLOCKERS:\n"
|
|
"VALIDATION:\n"
|
|
"LAST_UPDATED_BY:\n"
|
|
)
|
|
|
|
|
|
def _build_correction_message(
|
|
*,
|
|
missing_fields: list[str],
|
|
vague_fields: list[str],
|
|
extra_reasons: list[str],
|
|
context: str,
|
|
) -> str:
|
|
parts = [
|
|
"Canonical comment validation failed (fail closed before posting).",
|
|
]
|
|
if missing_fields:
|
|
parts.append("Missing fields: " + ", ".join(missing_fields) + ".")
|
|
if vague_fields:
|
|
parts.append("Vague or invalid fields: " + ", ".join(vague_fields) + ".")
|
|
parts.extend(extra_reasons)
|
|
parts.append("Fill the suggested template and retry.")
|
|
parts.append("Suggested template:\n" + _suggested_template(context))
|
|
return "\n".join(parts)
|
|
|
|
|
|
def assess_canonical_comment(
|
|
body: str,
|
|
*,
|
|
context: str | None = None,
|
|
force_workflow: bool = False,
|
|
) -> dict[str, Any]:
|
|
"""Validate outgoing comment text before a Gitea mutation."""
|
|
text = (body or "").strip()
|
|
ctx = infer_comment_context(text, explicit=context)
|
|
|
|
if not text:
|
|
return {
|
|
"allowed": True,
|
|
"is_workflow_comment": False,
|
|
"context": ctx,
|
|
"missing_fields": [],
|
|
"vague_fields": [],
|
|
"correction_message": "",
|
|
"suggested_template": "",
|
|
}
|
|
|
|
if _is_machine_generated(text):
|
|
return {
|
|
"allowed": True,
|
|
"is_workflow_comment": False,
|
|
"context": ctx,
|
|
"missing_fields": [],
|
|
"vague_fields": [],
|
|
"correction_message": "",
|
|
"suggested_template": "",
|
|
}
|
|
|
|
workflow = force_workflow or is_workflow_changing_comment(text)
|
|
if not workflow:
|
|
return {
|
|
"allowed": True,
|
|
"is_workflow_comment": False,
|
|
"context": ctx,
|
|
"missing_fields": [],
|
|
"vague_fields": [],
|
|
"correction_message": "",
|
|
"suggested_template": "",
|
|
}
|
|
|
|
fields = _parse_fields(text)
|
|
required = _required_fields_for_context(ctx)
|
|
missing = [name for name in required if not (fields.get(name) or "").strip()]
|
|
|
|
vague: list[str] = []
|
|
extra: list[str] = []
|
|
|
|
who = (fields.get("WHO_IS_NEXT") or "").strip().lower()
|
|
if who and who not in VALID_ROLES:
|
|
vague.append("WHO_IS_NEXT")
|
|
extra.append(
|
|
f"WHO_IS_NEXT must be one of: {', '.join(sorted(VALID_ROLES))}."
|
|
)
|
|
|
|
next_action = fields.get("NEXT_ACTION") or ""
|
|
if next_action and _is_vague_next_action(next_action):
|
|
vague.append("NEXT_ACTION")
|
|
|
|
next_prompt = fields.get("NEXT_PROMPT") or ""
|
|
if not _next_prompt_ok(next_prompt):
|
|
if "NEXT_PROMPT" not in missing:
|
|
vague.append("NEXT_PROMPT")
|
|
|
|
state = _state_value(fields)
|
|
blockers = (fields.get("BLOCKERS") or "").strip()
|
|
if "blocked" in state:
|
|
if not blockers or blockers.lower() in {"none", "n/a"}:
|
|
missing.append("BLOCKERS (unblock condition)")
|
|
elif not _UNBLOCK_RE.search(blockers):
|
|
vague.append("BLOCKERS")
|
|
extra.append("BLOCKED state requires an explicit unblock condition in BLOCKERS.")
|
|
|
|
if "superseded" in state:
|
|
canon = (fields.get("CANONICAL_ITEM") or fields.get("SUPERSEDED_BY") or "").strip()
|
|
superseded = (fields.get("SUPERSEDED_ITEM") or fields.get("SUPERSEDES") or "").strip()
|
|
if not canon and "CANONICAL_ITEM" not in missing:
|
|
missing.append("CANONICAL_ITEM")
|
|
if not superseded and "SUPERSEDED_ITEM" not in missing:
|
|
missing.append("SUPERSEDED_ITEM")
|
|
|
|
if "ready-to-merge" in state or "ready to merge" in state:
|
|
head_sha = fields.get("HEAD_SHA") or ""
|
|
merge_ready = fields.get("MERGE_READY") or ""
|
|
review_status = fields.get("REVIEW_STATUS") or ""
|
|
validation = fields.get("VALIDATION") or ""
|
|
proof_blob = " ".join((head_sha, merge_ready, review_status, validation))
|
|
has_sha = bool(_FULL_SHA_RE.search(proof_blob) or _SHORT_SHA_RE.search(proof_blob))
|
|
has_approval = bool(_APPROVAL_PROOF_RE.search(proof_blob))
|
|
if not has_sha or not has_approval:
|
|
extra.append(
|
|
"ready-to-merge STATE requires approval proof and HEAD_SHA in "
|
|
"REVIEW_STATUS, MERGE_READY, HEAD_SHA, or VALIDATION."
|
|
)
|
|
|
|
if ctx in ("pr_comment", "pr_review"):
|
|
head_sha = (fields.get("HEAD_SHA") or "").strip()
|
|
if not head_sha or not _SHORT_SHA_RE.search(head_sha):
|
|
if "HEAD_SHA" not in missing:
|
|
missing.append("HEAD_SHA")
|
|
|
|
if ctx == "issue_comment" and _PR_REF_RE.search(text):
|
|
related = (fields.get("RELATED_PRS") or "").strip()
|
|
if not related or related.lower() in {"none", "n/a", "-"}:
|
|
missing.append("RELATED_PRS")
|
|
|
|
# #695 AC7: untrusted / offline approval claims cannot certify merger handoff.
|
|
try:
|
|
import review_quarantine as _rq
|
|
|
|
for claim_reason in _rq.assess_untrusted_canonical_approval_claim(text):
|
|
extra.append(claim_reason)
|
|
except Exception:
|
|
# Fail closed on import/runtime errors for approval-shaped claims only.
|
|
lower = text.lower()
|
|
if (
|
|
"state:\napproved" in lower
|
|
or "who_is_next:\nmerger" in lower
|
|
or "merge_ready: true" in lower
|
|
or "merge_ready:\ntrue" in lower
|
|
or "ready-to-merge" in lower
|
|
):
|
|
extra.append(
|
|
"canonical approval/merge-ready claim could not be verified "
|
|
"for native review proof (#695 AC7; fail closed)"
|
|
)
|
|
|
|
allowed = not missing and not vague and not extra
|
|
correction = ""
|
|
if not allowed:
|
|
correction = _build_correction_message(
|
|
missing_fields=missing,
|
|
vague_fields=vague,
|
|
extra_reasons=extra,
|
|
context=ctx,
|
|
)
|
|
|
|
return {
|
|
"allowed": allowed,
|
|
"is_workflow_comment": True,
|
|
"context": ctx,
|
|
"missing_fields": missing,
|
|
"vague_fields": vague,
|
|
"extra_reasons": extra,
|
|
"correction_message": correction,
|
|
"suggested_template": _suggested_template(ctx) if not allowed else "",
|
|
} |