Files
Gitea-Tools/runtime_recovery_guard.py
T
jcwalker3andClaude Opus 4.8 6b58f04d39 fix(guard): split on & and unwrap subshells so real daemon kills classify (Closes #787)
`runtime_recovery_guard._SEGMENT_SPLIT_RE` claimed to split a compound command
line "on shell separators", but omitted the background-job separator `&`, and no
caller stripped a subshell wrapper before tokenising. `_analyse_kill_segment`
only inspects the first command token of a segment, so in both forms the kill
verb never reached the classifier.

Measured against the previous implementation at 35e94e10:

    sleep 1 & pkill -f mcp_server.py   ->  process_kill False, contamination False
    (pkill -f mcp_server.py)           ->  process_kill False, contamination False

Changes:

- Add `&` to `_SEGMENT_SPLIT_RE` as `(?:\|\||&&|[|;&\n])`; the two-character
  logical forms stay first so `&&` and `||` are consumed whole.
- Add `_strip_subshell()` and apply it in `_split_segments()`, removing leading
  `(` and trailing `)` (including nested wrappers) before tokenising.

Both forms now classify as contamination with reason class `manual_daemon_kill`,
and the record tool writes a durable marker for each. Direct-command behaviour is
unchanged. The contamination model, gated-task set, marker lifecycle, and
reconciler-only clearing path are untouched.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-21 19:56:00 -05:00

520 lines
20 KiB
Python

"""Fail-closed guard against manual MCP daemon process killing (#630).
Workflow recovery must use sanctioned reconnect/restart paths only: host
auto-reconnect, an operator-owned restart, or the documented client relaunch. A
session that instead runs ``pkill -f mcp_server.py`` has manipulated the very
host processes its own proof depends on.
Incident origin: a session ran ``ps aux | grep mcp_server``, then
``pkill -f mcp_server.py``, waited for the IDE to respawn the daemons, called
MCP tools, and closed issue #601. Nothing distinguished that closure from one
performed over a sanctioned runtime, and unrelated namespaces may have been
killed as collateral damage.
Partial detection already existed — ``native_mcp_preference.classify_command_path``
flags ``kill``/``pkill`` near ``mcp_server`` as an MCP-server touch, and
``review_workflow_boundary`` classifies a pre-review ``pkill`` as MCP repair
activity — but neither wrote a durable marker nor failed closed on the
review / merge / close mutations that followed.
This module mirrors ``stable_branch_push_guard`` (#671) deliberately: same
contamination-marker shape, same gated-task set, same reconciler-only clear.
Like that guard it is **pure** — callers gather the raw facts (the proposed
command line, known MCP pids, the durable marker, the process environment) and
pass them in, so one implementation serves prompts, MCP gates and tests.
Nothing here kills, spawns or inspects a process, performs I/O, or reads
durable state.
Design rules honoured (from the #630 acceptance criteria):
* Detect ``pkill -f mcp_server.py``, ``pkill -f gitea_mcp_server``, broad
``pkill -f mcp``, ``killall`` equivalents, and ``kill <pid>`` of a known MCP
daemon pid.
* Detect a pattern broad enough to take unrelated namespaces as collateral
damage (``pkill -f python``) even when it never names MCP.
* Never flag read-only inspection (``ps aux | grep mcp_server``), a sanctioned
client reconnect, or process management unrelated to the daemons. A bare
``kill <pid>`` with no MCP linkage is reported as *ambiguous*, never as
contamination, so ordinary subprocess work is not false-blocked.
* Never accept operator authorization from a tool argument. Authorization is
read from the process environment only — which an in-session worker cannot
set for an already-running daemon. A self-assertable ``operator_authorized``
argument was rejected in the PR #710 review (finding F1) and is not
reintroduced here.
* Contamination is never clearable by the same worker session; only a
reconciler (audit) role may clear it or bypass the gate.
"""
from __future__ import annotations
import os
import re
from typing import Any, Iterable
# Single source of truth for both the redactor and the gated-mutation set: the
# #671 guard already owns them, so the two contamination models can never drift
# apart on which mutations a contaminated session may still perform.
from stable_branch_push_guard import ( # noqa: F401 (CONTAMINATION_GATED_TASKS re-exported)
CONTAMINATION_GATED_TASKS,
redact_command,
)
CONTAMINATION_KIND = "manual_daemon_kill"
#: The session killed (or pattern-matched) an MCP daemon process directly.
REASON_MANUAL_DAEMON_KILL = "manual_daemon_kill"
#: The pattern was broad enough to sweep unrelated MCP namespaces.
REASON_BROAD_PROCESS_KILL = "broad_process_kill"
#: Operator authorization is read from this environment variable ONLY. It is
#: set outside the workflow session by the operator who owns host maintenance;
#: an in-session worker cannot set it for an already-running daemon. The value
#: is an audit reference (ticket, change id, or operator note) and is recorded
#: on the marker. Never accept this from a tool argument (#710 finding F1).
OPERATOR_AUTHORIZATION_ENV = "GITEA_OPERATOR_DAEMON_MAINTENANCE_AUTHORIZATION"
REMEDIATION = (
"Manual MCP daemon process killing is not a sanctioned workflow recovery. "
"Stop, leave the host processes alone, and recover through the client "
"reconnect / relaunch path (see docs/mcp-namespace-eof-recovery.md) or an "
"operator-owned restart. This session is workflow-contaminated until a "
"reconciler audits it; review, merge, close and completion mutations fail "
"closed until then."
)
# ── command tokenising ────────────────────────────────────────────────────────
# Split a compound command line into simple commands on shell separators so
# ``ps aux | grep mcp_server`` is analysed segment by segment and its harmless
# inspection half never reaches the kill classifier. The background separator
# ``&`` is a separator too: without it ``sleep 1 & pkill -f mcp_server.py`` was
# a single segment whose command position held ``sleep``, so the kill was never
# classified (#787). The two-character logical forms are listed first so ``&&``
# and ``||`` are consumed whole rather than split into single characters.
_SEGMENT_SPLIT_RE = re.compile(r"(?:\|\||&&|[|;&\n])")
_KILL_VERBS = frozenset({"kill", "pkill", "killall"})
# Tokens that may legitimately precede the kill verb in command position.
_COMMAND_PREFIXES = frozenset({
"sudo", "command", "exec", "time", "nohup", "env", "builtin",
})
# Matches the daemon process names: ``mcp_server``/``mcp-server`` (optionally
# ``gitea_``-prefixed, optionally ``.py``) or a standalone ``mcp`` token.
# ``mcpfoo`` deliberately does not match.
_MCP_TARGET_RE = re.compile(
r"(?:gitea[_-])?mcp[_-]?server|(?<![\w-])mcp(?![\w-])",
re.IGNORECASE,
)
# Patterns broad enough that matching them would kill unrelated MCP namespaces
# (and unrelated tooling) as collateral damage.
_BROAD_PATTERN_RE = re.compile(
r"^(?:python[\d.]*|node|uv|venv|java|ruby|perl|\.|\.\*|\*|%)$",
re.IGNORECASE,
)
# ``pkill``/``killall`` flags that consume the following token as their value,
# so it is not mistaken for a process pattern.
_VALUE_FLAGS = frozenset({
"-u", "-U", "-g", "-G", "-P", "-t", "-s", "-F", "-M", "-N", "-r",
"--signal", "--uid", "--euid", "--group", "--parent", "--session",
"--terminal", "--ns", "--nslist", "--pidfile", "--older",
})
# Sanctioned recovery language — informational only. Its presence never
# suppresses a detected kill; a session that describes a reconnect *and* runs
# ``pkill`` is still contaminated.
_SANCTIONED_RECOVERY_RE = re.compile(
r"/mcp\s+reconnect|client\s+reconnect|reconnect\s+the\s+(?:ide|client)|"
r"relaunch\s+the\s+(?:ide|client)|ide\s+restart|operator[- ]owned\s+restart",
re.IGNORECASE,
)
def _clean(value: str | None) -> str:
return (value or "").strip()
def _strip_subshell(segment: str) -> str:
"""Remove subshell wrappers so ``(pkill -f mcp_server.py)`` is classified.
The parentheses are shell syntax, not part of the simple command, so a
wrapped kill otherwise put ``(pkill`` in command position and never
reached the kill classifier (#787). Nested wrappers are unwrapped too.
"""
stripped = segment.strip()
while stripped.startswith("("):
stripped = stripped[1:].strip()
while stripped.endswith(")"):
stripped = stripped[:-1].strip()
return stripped
def _split_segments(command: str) -> list[str]:
segments = (_strip_subshell(seg) for seg in _SEGMENT_SPLIT_RE.split(command))
return [seg for seg in segments if seg]
def is_sanctioned_recovery(text: str | None) -> bool:
"""True when *text* describes a sanctioned reconnect/restart path.
Informational only: this never downgrades a detected process kill.
"""
return bool(_SANCTIONED_RECOVERY_RE.search(_clean(text)))
# ── kill classification ───────────────────────────────────────────────────────
def _analyse_kill_segment(
segment: str,
*,
mcp_pids: frozenset[str],
) -> dict[str, Any] | None:
"""Classify one command segment, or return None when it is not a kill."""
tokens = segment.split()
idx = 0
# Skip env assignments and harmless command prefixes (``sudo pkill ...``).
while idx < len(tokens) and (tokens[idx] in _COMMAND_PREFIXES or "=" in tokens[idx]):
idx += 1
if idx >= len(tokens):
return None
verb = os.path.basename(tokens[idx]).lower()
if verb not in _KILL_VERBS:
return None
operands: list[str] = []
skip_next = False
for token in tokens[idx + 1:]:
if skip_next:
skip_next = False
continue
if token.startswith("-"):
if token in _VALUE_FLAGS:
skip_next = True
continue
operands.append(token)
names_mcp = bool(_MCP_TARGET_RE.search(segment))
def _result(
*,
reason_class: str | None,
contamination: bool,
ambiguous: bool,
reason: str,
) -> dict[str, Any]:
return {
"verb": verb,
"operands": operands,
"reason_class": reason_class,
"contamination": contamination,
"ambiguous": ambiguous,
"reason": reason,
}
if verb in {"pkill", "killall"}:
if names_mcp:
return _result(
reason_class=REASON_MANUAL_DAEMON_KILL,
contamination=True,
ambiguous=False,
reason=(
f"'{verb}' targets the MCP daemon process pattern; this is "
"manual daemon killing, not a sanctioned recovery"
),
)
broad = [op for op in operands if _BROAD_PATTERN_RE.match(op)]
if broad:
return _result(
reason_class=REASON_BROAD_PROCESS_KILL,
contamination=True,
ambiguous=False,
reason=(
f"'{verb}' pattern {broad[0]!r} is broad enough to kill "
"unrelated MCP namespaces as collateral damage"
),
)
if not operands:
return _result(
reason_class=None,
contamination=False,
ambiguous=True,
reason=f"'{verb}' with no resolvable pattern; target unknown",
)
return _result(
reason_class=None,
contamination=False,
ambiguous=False,
reason=(
f"'{verb}' targets {operands!r}, which does not name an MCP "
"daemon or a broad pattern"
),
)
# ``kill`` — pid-addressed.
pids = [op for op in operands if op.isdigit()]
hits = sorted(set(pids) & mcp_pids, key=int)
if hits:
return _result(
reason_class=REASON_MANUAL_DAEMON_KILL,
contamination=True,
ambiguous=False,
reason=(
"'kill' targets known MCP daemon pid(s) "
f"{', '.join(hits)}; this is manual daemon killing"
),
)
if names_mcp:
return _result(
reason_class=REASON_MANUAL_DAEMON_KILL,
contamination=True,
ambiguous=False,
reason="'kill' resolves its target from an MCP daemon process lookup",
)
if not pids:
return _result(
reason_class=None,
contamination=False,
ambiguous=True,
reason="'kill' with no resolvable numeric pid; target unknown",
)
return _result(
reason_class=None,
contamination=False,
ambiguous=True,
reason=(
f"'kill' targets pid(s) {', '.join(pids)}, which are not known MCP "
"daemon pids; pass mcp_pids to resolve the ambiguity"
),
)
def classify_recovery_command(
command: str | None = None,
*,
mcp_pids: Iterable[Any] | None = None,
) -> dict[str, Any]:
"""Classify a proposed command for manual MCP daemon kill intent (#630).
Pure classification; operator authorization is applied separately by
:func:`assess_recovery_command`.
"""
text = _clean(command)
pid_set = frozenset(
str(pid).strip() for pid in (mcp_pids or []) if str(pid).strip()
)
segments: list[dict[str, Any]] = []
for raw_segment in _split_segments(text):
analysed = _analyse_kill_segment(raw_segment, mcp_pids=pid_set)
if analysed is not None:
analysed["segment"] = redact_command(raw_segment)
segments.append(analysed)
contaminating = [seg for seg in segments if seg["contamination"]]
return {
"command_present": bool(text),
"redacted_command": redact_command(text),
"process_kill": bool(segments),
"contamination": bool(contaminating),
"reason_class": contaminating[0]["reason_class"] if contaminating else None,
"ambiguous": bool(
not contaminating and any(seg["ambiguous"] for seg in segments)
),
"sanctioned_recovery": is_sanctioned_recovery(text),
"segments": segments,
"reasons": [seg["reason"] for seg in segments],
"known_mcp_pids": sorted(pid_set, key=lambda p: int(p) if p.isdigit() else 0),
}
# ── operator authorization ────────────────────────────────────────────────────
def operator_authorization(env: dict[str, str] | None = None) -> dict[str, Any]:
"""Read operator authorization for host daemon maintenance (#630 non-goal 1).
Authorization comes from :data:`OPERATOR_AUTHORIZATION_ENV` in the process
environment and from nowhere else. A worker session cannot set an
environment variable for an already-running daemon, so this cannot be
self-asserted the way a tool argument could be (#710 finding F1).
"""
source = env if env is not None else os.environ
reference = _clean(source.get(OPERATOR_AUTHORIZATION_ENV))
return {
"authorized": bool(reference),
"reference": reference or None,
"source": OPERATOR_AUTHORIZATION_ENV if reference else None,
"self_assertable": False,
}
def assess_recovery_command(
command: str | None = None,
*,
mcp_pids: Iterable[Any] | None = None,
env: dict[str, str] | None = None,
) -> dict[str, Any]:
"""Classify *command* and apply operator authorization (#630 AC1/AC2)."""
classification = classify_recovery_command(command, mcp_pids=mcp_pids)
authorization = operator_authorization(env)
detected = classification["contamination"]
contaminated = detected and not authorization["authorized"]
return {
"classification": classification,
"authorization": authorization,
"contaminated": contaminated,
"authorized_bypass": bool(detected and authorization["authorized"]),
"remediation": REMEDIATION if contaminated else None,
}
# ── contamination record + gate ───────────────────────────────────────────────
def build_contamination_record(
*,
reason_class: str,
command_redacted: str | None = None,
session_id: str | None = None,
remote: str | None = None,
role: str | None = None,
detail: str | None = None,
authorization_reference: str | None = None,
) -> dict[str, Any]:
"""Build the durable contamination marker payload (redacted, audit-safe).
``reason_class`` is :data:`REASON_MANUAL_DAEMON_KILL` or
:data:`REASON_BROAD_PROCESS_KILL`. The command is stored already redacted;
secrets never persist on the marker.
"""
return {
"kind": CONTAMINATION_KIND,
"reason_class": _clean(reason_class) or REASON_MANUAL_DAEMON_KILL,
"command_summary": redact_command(command_redacted),
"session_id": _clean(session_id) or None,
"remote": _clean(remote) or None,
"role": _clean(role) or None,
"detail": _clean(detail) or None,
"authorization_reference": _clean(authorization_reference) or None,
"cleared_by_reconciler": False,
}
def assess_contamination_gate(
marker: dict[str, Any] | None,
*,
task: str | None,
actual_role: str | None,
) -> dict[str, Any]:
"""Fail closed on gated mutations while a contamination marker is live (#630 AC3).
* No marker → allowed.
* Reconciler (audit) role → allowed (the sanctioned inspect/clear path).
* Marker present + ``task`` in :data:`CONTAMINATION_GATED_TASKS` → blocked.
* Marker present + non-gated task (``comment_issue``, ``lock_issue``) →
allowed, so the contaminated worker can still post the durable audit
comment and hand off.
"""
if not marker or marker.get("cleared_by_reconciler"):
return {"block": False, "reasons": [], "task": task}
role = _clean(actual_role).lower()
if role == "reconciler":
return {
"block": False,
"reasons": [],
"task": task,
"detail": "reconciler audit path is exempt from the contamination gate",
}
task_name = _clean(task)
if task_name and task_name in CONTAMINATION_GATED_TASKS:
summary = marker.get("command_summary") or marker.get("detail") or "(no summary)"
reason_class = marker.get("reason_class") or REASON_MANUAL_DAEMON_KILL
return {
"block": True,
"reasons": [
f"session is workflow-contaminated ({reason_class}): {summary}. "
f"'{task_name}' is blocked until a reconciler audits and clears "
"the contamination. " + REMEDIATION
],
"task": task_name,
}
return {"block": False, "reasons": [], "task": task_name or None}
def format_contamination_gate_error(gate: dict[str, Any]) -> str:
"""Single RuntimeError message for MCP mutation gates."""
reasons = "; ".join(gate.get("reasons") or ["session workflow-contaminated"])
return f"Runtime-recovery contamination gate (#630): {reasons}"
# ── final-report rules ────────────────────────────────────────────────────────
# Claims that assert a clean session. While a marker is live these are false and
# must be rejected rather than merely downgraded.
_CLEAN_CLAIM_RE = re.compile(
r"\bclean\s+session\b|\bsession\s+(?:is|was|remains)\s+clean\b|"
r"\bno\s+contamination\b|\buncontaminated\b|\bcontamination\s*[:=]\s*none\b|"
r"\bworkflow[- ]clean\b|\bno\s+workflow\s+contamination\b",
re.IGNORECASE,
)
# Language that actually surfaces the contamination to a reader.
_SURFACED_RE = re.compile(
r"manual[_ ]daemon[_ ]kill|broad[_ ]process[_ ]kill|daemon\s+process\s+kill|"
r"contaminated\s+recovery|runtime[- ]recovery\s+contamination|"
r"workflow[- ]contaminated",
re.IGNORECASE,
)
def assess_final_report_claim(
report_text: str | None,
marker: dict[str, Any] | None,
) -> dict[str, Any]:
"""Reject clean-session claims while contaminated (#630 scope item 4).
A live marker imposes two obligations on the final report: it must surface
the contaminated recovery explicitly, and it must not claim the session is
clean. Either failure blocks.
"""
if not marker or marker.get("cleared_by_reconciler"):
return {
"block": False,
"reasons": [],
"contaminated": False,
"surfaced": None,
"clean_claim": False,
}
text = _clean(report_text)
surfaced = bool(_SURFACED_RE.search(text))
clean_claim = bool(_CLEAN_CLAIM_RE.search(text))
reasons: list[str] = []
if clean_claim:
reasons.append(
"final report claims a clean session while a live "
f"{marker.get('reason_class') or CONTAMINATION_KIND} contamination "
"marker exists; the claim is false and must be removed"
)
if not surfaced:
reasons.append(
"final report does not surface the contaminated runtime recovery; "
"the report must state that MCP daemon processes were manually "
"killed and that the session awaits a reconciler audit"
)
return {
"block": bool(reasons),
"reasons": reasons,
"contaminated": True,
"surfaced": surfaced,
"clean_claim": clean_claim,
"reason_class": marker.get("reason_class"),
}