Compare commits

..
Author SHA1 Message Date
sysadmin a1e5c33bfc 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.
2026-07-13 05:57:42 -04:00
43 changed files with 294 additions and 13034 deletions
-9
View File
@@ -55,12 +55,3 @@ GITEA_MCP_PROFILE=prgs
# GITEA_MERGER_WORKTREE=/path/to/repo/branches/merge-pr456
# GITEA_RECONCILER_WORKTREE=/path/to/repo/branches/reconcile-pr456
# GITEA_ACTIVE_WORKTREE=/path/to/repo/branches/session-override
# Durable HMAC key for irrecoverable decision-lock provenance artifacts (#709 F4).
# REQUIRED in production native MCP processes that mint or verify recovery
# authorization (reconciler + merger must share the same key). Hex (preferred),
# base64, or utf-8 literal (>=16 bytes). Never generate an ephemeral per-process
# key — cross-process / post-restart verification would fail closed.
# GITEA_IRRECOVERABLE_AUTH_HMAC_KEY=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef
# Optional key version string bound into the signature (for rotation).
# GITEA_IRRECOVERABLE_AUTH_HMAC_KEY_VERSION=v1
-505
View File
@@ -7,19 +7,6 @@ from typing import Any
PROTECTED_BRANCHES = frozenset({"master", "main", "dev"})
# Evidence / preservation branches must never be removed by cleanup tools
# (e.g. chore/issue-681-preserve-review-session-wip).
_PRESERVATION_MARKERS = ("preserve", "preservation", "evidence")
def is_preservation_or_evidence_branch(branch: str | None) -> bool:
"""Return True when *branch* is a preservation/evidence ref that must stay."""
if not branch:
return False
name = str(branch).lower()
return any(marker in name for marker in _PRESERVATION_MARKERS)
_RAW_BRANCH_DELETE_PATTERNS = (
re.compile(r"\bgit(?:\s+-C\s+\S+)?\s+branch\s+-[dD]\b[^\n\r]*", re.I),
re.compile(r"\bgit(?:\s+-C\s+\S+)?\s+push\b[^\n\r]*\s--delete\b[^\n\r]*", re.I),
@@ -85,11 +72,6 @@ def assess_merged_pr_branch_cleanup(
reasons.append("PR head branch is missing")
if head_branch in protected:
reasons.append(f"branch '{head_branch}' is protected")
if is_preservation_or_evidence_branch(head_branch):
reasons.append(
f"branch '{head_branch}' is a preservation/evidence branch and "
"cannot be deleted through merged-PR cleanup"
)
if head_branch in open_pr_heads:
reasons.append("an open PR still references this head branch")
if head_on_target is False:
@@ -114,490 +96,3 @@ def assess_merged_pr_branch_cleanup(
"block_reasons": reasons,
"recommended_action": "delete_remote_branch" if safe else "keep_remote_branch",
}
# ---------------------------------------------------------------------------
# #687 remediation: post-delete readback + active ownership protection
# ---------------------------------------------------------------------------
READBACK_NOT_FOUND = "not_found"
READBACK_EXISTS = "exists"
READBACK_AUTHENTICATION = "authentication_error"
READBACK_AUTHORIZATION = "authorization_error"
READBACK_TRANSPORT = "transport_error"
READBACK_UNEXPECTED = "unexpected_response"
READBACK_AMBIGUOUS_404 = "ambiguous_not_found"
# Scope of a 404: only branch-scoped absence may set verified_absent.
NOT_FOUND_SCOPE_BRANCH = "branch"
NOT_FOUND_SCOPE_REPOSITORY = "repository"
NOT_FOUND_SCOPE_HOST = "host"
NOT_FOUND_SCOPE_UNKNOWN = "unknown"
ERROR_CLASS_AUTHENTICATION = "authentication"
ERROR_CLASS_AUTHORIZATION = "authorization"
ERROR_CLASS_TRANSPORT = "transport"
ERROR_CLASS_UNEXPECTED = "unexpected"
OWNERSHIP_CATEGORY_AUTHOR_SESSION = "author_session"
OWNERSHIP_CATEGORY_AUTHOR_LEASE = "author_lease"
OWNERSHIP_CATEGORY_REVIEWER_LEASE = "reviewer_lease"
OWNERSHIP_CATEGORY_MERGER_LEASE = "merger_lease"
OWNERSHIP_CATEGORY_CONTROLLER_LEASE = "controller_lease"
OWNERSHIP_CATEGORY_RECONCILER_LEASE = "reconciler_lease"
OWNERSHIP_CATEGORY_WORKTREE_BINDING = "worktree_binding"
OWNERSHIP_CATEGORY_INVENTORY_ERROR = "ownership_inventory_error"
_ROLE_TO_OWNERSHIP_CATEGORY = {
"author": OWNERSHIP_CATEGORY_AUTHOR_LEASE,
"reviewer": OWNERSHIP_CATEGORY_REVIEWER_LEASE,
"merger": OWNERSHIP_CATEGORY_MERGER_LEASE,
"controller": OWNERSHIP_CATEGORY_CONTROLLER_LEASE,
"reconciler": OWNERSHIP_CATEGORY_RECONCILER_LEASE,
}
_ACTIVE_OWNERSHIP_STATUSES = frozenset(
{"active", "live", "claimed", "in_progress", "working", "pushing", "pushed"}
)
_TERMINAL_OWNERSHIP_STATUSES = frozenset(
{"released", "abandoned", "done", "blocked", "terminal", "closed"}
)
_EXPIRED_STATUSES = frozenset({"expired"})
_STALE_STATUSES = frozenset({"stale", "stale_dead_process", "stale_missing_worktree"})
def _norm_str(value: Any) -> str:
return str(value or "").strip()
def normalize_host(host: str | None) -> str:
"""Normalize a host identity for ownership matching (no credentials)."""
text = _norm_str(host).lower()
for prefix in ("https://", "http://"):
if text.startswith(prefix):
text = text[len(prefix) :]
# Drop path/query if a full URL slipped through.
text = text.split("/", 1)[0]
text = text.split("?", 1)[0]
return text.rstrip(".")
def ownership_category_for_role(role: str | None) -> str:
"""Map a role kind to a non-secret ownership category label."""
key = _norm_str(role).lower()
return _ROLE_TO_OWNERSHIP_CATEGORY.get(key, f"{key or 'unknown'}_lease")
def classify_branch_readback_http_status(
status_code: int | None,
*,
not_found_scope: str | None = None,
) -> dict[str, Any]:
"""Classify a GET-branch HTTP status into a secret-free readback result.
R1: A bare/generic/repository/wrong-host 404 never yields
``verified_absent=True``. Only an authoritative *branch-scoped* not-found
(``not_found_scope='branch'``) may verify deletion.
"""
if status_code == 404:
scope = _norm_str(not_found_scope).lower() or NOT_FOUND_SCOPE_UNKNOWN
if scope == NOT_FOUND_SCOPE_BRANCH:
return {
"status": READBACK_NOT_FOUND,
"error_class": None,
"verified_absent": True,
"branch_present": False,
"not_found_scope": NOT_FOUND_SCOPE_BRANCH,
"reasons": [],
}
# repository / host / unknown / generic 404 — not verified absence
reason = {
NOT_FOUND_SCOPE_REPOSITORY: (
"post-delete readback 404 is repository-scoped, not branch absence"
),
NOT_FOUND_SCOPE_HOST: (
"post-delete readback 404 is host-scoped, not branch absence"
),
}.get(
scope,
"post-delete readback 404 is ambiguous (not branch-scoped); "
"cannot verify absence",
)
return {
"status": READBACK_AMBIGUOUS_404,
"error_class": ERROR_CLASS_UNEXPECTED,
"verified_absent": False,
"branch_present": None,
"not_found_scope": scope,
"reasons": [reason],
}
if status_code in (401, 407):
return {
"status": READBACK_AUTHENTICATION,
"error_class": ERROR_CLASS_AUTHENTICATION,
"verified_absent": False,
"branch_present": None,
"reasons": ["post-delete branch readback authentication failed"],
}
if status_code == 403:
return {
"status": READBACK_AUTHORIZATION,
"error_class": ERROR_CLASS_AUTHORIZATION,
"verified_absent": False,
"branch_present": None,
"reasons": ["post-delete branch readback authorization failed"],
}
if status_code is not None and 200 <= int(status_code) < 300:
return {
"status": READBACK_EXISTS,
"error_class": None,
"verified_absent": False,
"branch_present": True,
"reasons": ["post-delete readback found branch still present"],
}
if status_code is not None and int(status_code) >= 500:
return {
"status": READBACK_TRANSPORT,
"error_class": ERROR_CLASS_TRANSPORT,
"verified_absent": False,
"branch_present": None,
"reasons": ["post-delete branch readback transport/upstream failure"],
}
return {
"status": READBACK_UNEXPECTED,
"error_class": ERROR_CLASS_UNEXPECTED,
"verified_absent": False,
"branch_present": None,
"reasons": ["post-delete branch readback returned an unexpected response"],
"http_status": status_code,
}
def _extract_http_status(exc: BaseException) -> int | None:
"""Extract an HTTP status code from an exception chain (secret-free)."""
seen: set[int] = set()
current: BaseException | None = exc
while current is not None and id(current) not in seen:
seen.add(id(current))
for attr in ("code", "status", "status_code"):
value = getattr(current, attr, None)
if isinstance(value, int) and 100 <= value <= 599:
return value
text = str(current) if current is not None else ""
match = re.match(r"HTTP\s+(\d{3})\b", text)
if match:
return int(match.group(1))
current = current.__cause__ or current.__context__
return None
def classify_branch_readback_exception(
exc: BaseException,
*,
not_found_scope: str | None = None,
) -> dict[str, Any]:
"""Classify a GET-branch exception without leaking credentials or bodies.
R1: substring ``404`` / ``not found`` alone never becomes verified_absent.
Callers must pass ``not_found_scope='branch'`` only after authoritative
proof that the repository/host is still reachable and the 404 is branch-level.
"""
status_code = _extract_http_status(exc)
if status_code is None:
lower = str(exc).lower() if exc is not None else ""
if any(
token in lower
for token in (
"timed out",
"timeout",
"connection",
"network",
"temporarily unavailable",
"name or service not known",
"nodename nor servname",
)
):
return {
"status": READBACK_TRANSPORT,
"error_class": ERROR_CLASS_TRANSPORT,
"verified_absent": False,
"branch_present": None,
"reasons": [
"post-delete branch readback transport/upstream failure"
],
}
if "unauthorized" in lower:
status_code = 401
elif "forbidden" in lower:
status_code = 403
elif "404" in lower or "not found" in lower:
# Ambiguous: do NOT treat as branch absence without scope proof.
status_code = 404
if not_found_scope is None:
not_found_scope = NOT_FOUND_SCOPE_UNKNOWN
if status_code is not None:
return classify_branch_readback_http_status(
status_code, not_found_scope=not_found_scope
)
return {
"status": READBACK_UNEXPECTED,
"error_class": ERROR_CLASS_UNEXPECTED,
"verified_absent": False,
"branch_present": None,
"reasons": ["post-delete branch readback returned an unexpected response"],
}
def assess_post_delete_readback(readback: dict[str, Any] | None) -> dict[str, Any]:
"""Decide whether DELETE success may be reported after branch readback.
Success requires authoritative branch-scoped not-found
(``verified_absent=True``). DELETE HTTP success alone is never enough.
Generic/repository/host 404 cannot verify absence (R1).
"""
payload = dict(readback or {})
status = _norm_str(payload.get("status")) or READBACK_UNEXPECTED
# Only explicit verified_absent flag counts — never infer from status alone
# when status is a bare not_found without branch scope proof.
verified = bool(payload.get("verified_absent")) is True
if verified and status == READBACK_NOT_FOUND:
return {
"ok": True,
"success": True,
"verified_absent": True,
"readback": {
"status": READBACK_NOT_FOUND,
"verified_absent": True,
"branch_present": False,
"error_class": None,
"not_found_scope": NOT_FOUND_SCOPE_BRANCH,
},
"reasons": [],
}
reasons = list(payload.get("reasons") or [])
if not reasons:
if status == READBACK_EXISTS:
reasons = ["post-delete readback found branch still present"]
elif status == READBACK_AUTHENTICATION:
reasons = ["post-delete branch readback authentication failed"]
elif status == READBACK_AUTHORIZATION:
reasons = ["post-delete branch readback authorization failed"]
elif status == READBACK_TRANSPORT:
reasons = ["post-delete branch readback transport/upstream failure"]
elif status == READBACK_AMBIGUOUS_404:
reasons = [
"post-delete readback 404 is not branch-scoped; "
"cannot verify absence"
]
else:
reasons = ["post-delete branch readback could not verify deletion"]
return {
"ok": False,
"success": False,
"verified_absent": False,
"readback": {
"status": status,
"verified_absent": False,
"branch_present": payload.get("branch_present"),
"error_class": payload.get("error_class"),
"not_found_scope": payload.get("not_found_scope"),
},
"reasons": reasons,
"blocker_kind": "post_delete_readback_failed",
}
def cleanup_result_envelope(
*,
success: bool,
performed: bool,
delete_acknowledged: bool,
verified_absent: bool,
**extra: Any,
) -> dict[str, Any]:
"""R2: consistent top-level cleanup result fields on every return path."""
out: dict[str, Any] = {
"success": bool(success),
"performed": bool(performed),
"delete_acknowledged": bool(delete_acknowledged),
"verified_absent": bool(verified_absent),
}
out.update(extra)
return out
def _repo_matches(
record: dict[str, Any],
*,
remote: str,
org: str,
repo: str,
host: str | None = None,
) -> bool:
"""Match ownership record to target remote/org/repo and normalized host."""
if (
_norm_str(record.get("remote")).lower() != _norm_str(remote).lower()
or _norm_str(record.get("org")).lower() != _norm_str(org).lower()
or _norm_str(record.get("repo")).lower() != _norm_str(repo).lower()
):
return False
expected_host = normalize_host(host)
record_host = normalize_host(record.get("host") or record.get("host_name"))
# When both sides declare a host, they must agree after normalization.
# A record host that disagrees with the expected host is out of scope.
# Legacy records without host still match when remote/org/repo agree.
if expected_host and record_host and expected_host != record_host:
return False
return True
def _branch_matches(record: dict[str, Any], branch: str) -> bool:
return _norm_str(record.get("branch")) == _norm_str(branch)
def assess_ownership_record_activity(record: dict[str, Any]) -> dict[str, Any]:
"""Classify one ownership record as blocking or non-blocking.
Distinguishes active ownership from expired/released/terminal/stale records.
Expired/stale records block unless reclaim_allowed is *explicitly* True
(O2: never treat missing/unknown reclaim as auto-allowed).
"""
status = _norm_str(record.get("status")).lower()
category = _norm_str(record.get("category")) or "unknown"
reclaim_allowed = record.get("reclaim_allowed")
if category == OWNERSHIP_CATEGORY_INVENTORY_ERROR:
return {
"blocks": True,
"status": status or "unknown",
"category": category,
"reason": "ownership inventory failed closed",
}
if status in _TERMINAL_OWNERSHIP_STATUSES:
return {
"blocks": False,
"status": status,
"category": category,
"reason": f"{category} ownership is terminal/released ({status})",
}
if status in _ACTIVE_OWNERSHIP_STATUSES:
return {
"blocks": True,
"status": status,
"category": category,
"reason": f"active {category} ownership still uses the target branch",
}
if status in _EXPIRED_STATUSES or status in _STALE_STATUSES:
# O2: only explicit reclaim_allowed=True skips the block.
if reclaim_allowed is True:
return {
"blocks": False,
"status": status,
"category": category,
"reason": (
f"{category} ownership is {status} and reclaimable; "
"does not block deletion"
),
}
return {
"blocks": True,
"status": status,
"category": category,
"reason": (
f"{status} {category} ownership still protects the target "
"branch (reclaim not proven; fail closed)"
),
}
# Unknown status → fail closed
return {
"blocks": True,
"status": status or "unknown",
"category": category,
"reason": (
f"unclassified {category} ownership status "
f"'{status or 'unknown'}'; fail closed"
),
}
def assess_active_branch_ownership(
*,
remote: str,
org: str,
repo: str,
branch: str,
host: str | None = None,
records: list[dict[str, Any]] | None = None,
) -> dict[str, Any]:
"""Assess whether any active ownership still uses *branch* in *repo*.
Matching requires remote/org/repo/branch and, when provided, normalized
host identity. Other repositories, hosts, or branches never false-block.
"""
target_branch = _norm_str(branch)
expected_host = normalize_host(host)
considered: list[dict[str, Any]] = []
blocking: list[dict[str, Any]] = []
ignored: list[dict[str, Any]] = []
for raw in records or []:
if not isinstance(raw, dict):
continue
if not _repo_matches(
raw, remote=remote, org=org, repo=repo, host=expected_host or None
):
ignored.append(
{
"category": _norm_str(raw.get("category")) or "unknown",
"reason": "different repository or host scope",
}
)
continue
if not _branch_matches(raw, target_branch):
ignored.append(
{
"category": _norm_str(raw.get("category")) or "unknown",
"reason": "different branch",
}
)
continue
activity = assess_ownership_record_activity(raw)
entry = {
"category": activity["category"],
"status": activity["status"],
"blocks": activity["blocks"],
"reason": activity["reason"],
}
considered.append(entry)
if activity["blocks"]:
blocking.append(entry)
block = bool(blocking)
categories = sorted({b["category"] for b in blocking})
reasons = [
(
"active ownership protects branch "
f"'{target_branch}': " + "; ".join(b["reason"] for b in blocking)
)
] if block else []
return {
"block": block,
"safe_to_delete": not block,
"remote": remote,
"org": org,
"repo": repo,
"host": expected_host or None,
"branch": target_branch,
"blocking_categories": categories,
"blocking": blocking,
"considered": considered,
"ignored_out_of_scope": ignored,
"reasons": reasons,
"blocker_kind": "active_branch_ownership" if block else None,
"recommended_action": "keep_remote_branch" if block else "delete_remote_branch",
}
@@ -1,198 +0,0 @@
# ADR: Stable control runtime vs dev runtime (Gitea MCP)
- **Status:** Accepted (policy effective immediately for LLM sessions; tooling may lag)
- **Date:** 2026-07-09
- **Tracking issue:** [#615](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/615)
- **Related:**
- [#543](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/543) / `docs/mcp-namespace-health.md` — client-namespace health
- `docs/mcp-namespace-eof-recovery.md` — reconnect-only EOF recovery (no PID kill)
- [#558](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/558) / `docs/mcp-daemon-import-guard.md` — sanctioned daemon
- [#557](https://gitea.prgs.cc/Scaled-Tech-Consulting/Gitea-Tools/issues/557) / `docs/bootstrap-review-path.md` — controller bootstrap for self-hosted fixes
- Allocator / control-plane ADR: `docs/architecture/mcp-allocator-control-plane-observability-adr.md` (#613 / PR #614)
## 1. Context
The Gitea MCP server is the **control plane** for real issue/PR mutations (create, comment, lock, review, merge, etc.). When author/reviewer/merger/reconciler sessions kill or restart that process, relaunch it from a feature worktree, or edit the checkout that process loads, operators observe:
- Mid-session identity/preflight resets
- Stale-runtime vs master parity failures
- IDE transport EOF / “tool not found” while code on disk has changed
- Accidental production mutations from experimental code
This ADR separates **stable control runtime** from **dev/test runtime** and defines promotion proof.
## 2. Decision
### 2.1 Stable control runtime
The Gitea MCP server used for **real workflow mutations** is the **stable control runtime**.
Characteristics:
- Loads a known, promoted revision of Gitea-Tools (or the packaged release layout operators designate)
- Registered in the IDE/client as the production namespaces (`gitea-tools`, `gitea-reviewer`, `gitea-merger`, `gitea-reconciler`, etc.)
- Holds production profile credentials via sanctioned keychain/env paths only
### 2.2 Dev / test runtime
MCP **server code** development and testing:
- Happens in isolated **`branches/`** worktrees (or other non-stable checkouts)
- May use a **separate** dev/test MCP runtime/process when process-level testing is required
- **Must not** be used for real Gitea mutations on production issues/PRs
### 2.3 Forbidden actions (normal sessions)
Normal **author, reviewer, merger, and reconciler** LLM sessions **must not**:
| Forbidden | Why |
|-----------|-----|
| Kill the running MCP server process | Drops all concurrent sessions; loses preflight state |
| Restart / relaunch the MCP server process | Same as kill; causes stale/identity churn mid-workflow |
| Relaunch MCP from a development worktree | Runs unpromoted code against production mutations |
| Edit files in the stable runtime checkout | Hot-mutates control plane under concurrent users |
| Use experimental/dev MCP for real Gitea mutations | Bypasses promotion proof and audit expectations |
| Bypass or self-reset a stale master-parity gate | The gate is fail-closed; only an operator reload restores parity |
**LLM-allowed vs operator-owned (authoritative split):**
| Actor | May do | Must not do |
|-------|--------|-------------|
| **LLM session** | Call tools on the already-running stable namespaces; **client reconnect** after transport EOF (no process kill); pass `worktree_path` / role worktree args; report blockers and stop mutations when unhealthy/stale | Kill, restart, or relaunch any MCP process; bump config mtimes to force reload; edit the stable checkout; switch to a dev MCP for production mutations |
| **Operator / release-manager** | Supervised restart/reload of the **stable** control runtime; dual-namespace client configuration; §2.4 promotions; incident recovery | — |
EOF / transport recovery for LLM sessions: **client reconnect only** (see `docs/mcp-namespace-eof-recovery.md`). Do not “fix” health by killing PIDs or bumping MCP config mtimes as a normal session procedure.
This ADR **supersedes** any older runbook wording that told the LLM to relaunch or restart the client/MCP as a self-service step. Where `docs/llm-workflow-runbooks.md` (or wiki runbooks) discuss dual-namespace setup or workspace rebind, **process restart/relaunch is operator-owned**; the LLM stops, reports, and waits.
### 2.4 Promotion (operator / release-manager only)
Promotion of a new revision into the stable control runtime is an **explicit operator/release-manager action**, not an LLM self-service step.
A promotion **must record** (issue comment, release note, or promotion ledger):
| Field | Description |
|-------|-------------|
| **previous runtime SHA** | Commit previously loaded by stable runtime |
| **promoted runtime SHA** | Commit after promotion |
| **source branch/PR** | Where the change was reviewed |
| **restart/reload method** | How the process was cycled (e.g. supervised restart, client reload) |
| **health check proof** | Client-namespace probe success (`gitea_whoami` / namespace health) |
| **identity/profile proof** | Expected profile(s) and username(s) after reload |
| **workspace/root proof** | Stable checkout path / root matches intended layout |
| **mutation capability proof** | Required permissions for the target role present; forbidden ops still forbidden |
| **rollback instructions** | How to restore previous SHA and re-verify health |
Suggested durable marker:
```text
## MCP STABLE RUNTIME PROMOTION (#615)
Status: COMPLETED | ROLLED_BACK | ABORTED
Previous-SHA: <full sha>
Promoted-SHA: <full sha>
Source-PR: <number>
Source-Branch: <name>
Reload-Method: <text>
Health-Proof: client_namespace whoami OK / assess_mcp_namespace_health OK
Identity-Proof: profile=<name> user=<name>
Workspace-Proof: root=<path>
Mutation-Proof: allowed_ops include <…>; forbidden include <…>
Rollback: checkout <previous sha>; reload method <…>; re-run health/identity proofs
Operator: <username>
Timestamp: <ISO-8601>
```
### 2.5 Unhealthy stable runtime → stop work
If the stable MCP runtime is **unhealthy**, including any of:
- client-namespace probes fail
- wrong identity / wrong profile
- wrong workspace root
- missing mutation capability for the intended role
- persistent EOF after **client reconnect**
- **master parity is stale** (`startup_head` behind on-disk `master` / `restart_required` from the parity gate)
then:
1. **Normal PR / review / merge / issue-mutation work must stop immediately.**
2. The session **reports** the unhealthy/stale state (tool error, CTH, or operator handoff) with startup vs current head when known.
3. Do **not** improvise: no LLM process kill/restart, no dev-worktree MCP for production mutations, no env escape hatches, no manual gate bypass.
4. Resume only after:
- an **operator** restores the runtime (see §2.6 for routine post-merge parity reload), **or**
- a **controlled** promotion/rollback completes with the §2.4 promotion record, **or**
- a controller invokes the narrow **bootstrap review path** (#557) when the defect is self-hosted and documented,
- **and** the session re-verifies health (and master parity when applicable) before the next mutation.
### 2.6 Routine post-merge master-parity staleness (operator reload, not promotion)
**Symptom:** After merges land on `master`, a long-lived stable MCP process still runs the pre-merge `startup_head`. The master-parity gate marks the server **stale** / `restart_required` and **blocks mutations**.
**Sanctioned response (authoritative):**
| Step | Actor | Action |
|------|-------|--------|
| 1 | LLM session | Mutations stop immediately when the gate reports stale. |
| 2 | LLM session | Report the stale state (startup head, current master head, that operator reload is required). Do not retry mutations. |
| 3 | **Operator** | Reload/restart the **stable** control MCP so it loads current `master` (supervised client/daemon reload). |
| 4 | LLM session | Resume only after startup/current-head parity is verified (e.g. `gitea_get_runtime_context` / parity assessment shows in parity). |
**Not a §2.4 promotion:** Catching the already-designated stable control checkout up to a newly advanced `master` is a **routine operator reload** of the stable runtime. It does **not** require the nine-field promotion ledger. Use §2.4 only when **changing which unpromoted/dev revision** becomes the stable control runtime (new source branch/PR into the stable designation).
**Code note:** `master_parity_gate.py` may still say “restart the server” in machine-facing reason strings. That string names the **operator recovery action**, not an LLM self-service instruction. This ADR and the runbooks define the actor split.
## 3. Relationship to other controls
| Doc / mechanism | Interaction |
|-----------------|-------------|
| Namespace health (#543) | Proves IDE client can call tools; does not authorize restart |
| EOF recovery | Reconnect only; no process kill |
| Daemon import guard (#558) | Mutations require sanctioned daemon; not a bare shell import |
| Bootstrap path (#557) | Only controller-authorized exception when live runtime cannot review its own fix |
| Allocator / control-plane ADR | Coordination DB is separate; still depends on a healthy MCP surface for Gitea writes |
## 4. Consequences
### Positive
- Predictable control plane for concurrent LLMs
- Clear operator-only promotion gate with rollback
- Aligns session behavior with health/EOF docs already landed
### Costs
- LLM sessions must wait when runtime is sick (no DIY restart)
- Operators must maintain promotion discipline and dual-runtime config if they use a dev MCP
### Non-goals
- Does not ban operator-supervised restarts during incidents
- Does not replace CI or code review for MCP changes
- Does not authorize editing stable checkout “because tests need a quick fix”
## 5. Implementation follow-ups (optional tooling)
These may land in later issues; the **policy binds sessions now**:
1. Session preflight that refuses mutations if workspace root equals a `branches/` feature worktree configured as “dev only.”
2. Explicit `runtime_kind=stable|dev` in MCP config and `gitea_whoami` profile metadata.
3. Promotion checklist script that emits the durable promotion marker fields.
**Not optional (issue #615 acceptance criterion 2):** operator guide and runbooks **must** cross-link this ADR (see §6). Cross-links are documentation acceptance, not deferred tooling.
## 6. Acceptance for this ADR
1. Document merged under `docs/architecture/mcp-stable-control-runtime-policy-adr.md`.
2. **Operator guide / runbooks cross-link this ADR** (`docs/wiki/Operator-Guide.md`, `docs/wiki/Runbooks.md`, `docs/llm-workflow-runbooks.md`).
3. Issue #615 references this path.
4. LLM/operator runbooks treat kill/restart/relaunch-from-worktree as **LLM violations**; process restart is **operator-owned**.
5. Unhealthy runtime (including **stale master parity**) stops normal mutation work until operator restore/reload, promotion/rollback, or #557 bootstrap — then re-verify parity before mutating.
6. Routine post-merge parity reload is documented as operator reload (§2.6), not an LLM self-restart and not a full §2.4 promotion.
## 7. Document history
| Date | Change |
|------|--------|
| 2026-07-09 | Initial ADR: stable vs dev runtime, forbidden session actions, promotion proof fields, stop-work rule |
| 2026-07-16 | Review 443 remediation (#615 / PR #616): mandatory cross-links; LLM vs operator restart split; routine post-merge parity staleness (§2.6); stale parity in §2.5 unhealthy triggers |
-185
View File
@@ -238,43 +238,11 @@ narrow operation set:
- `gitea.issue.comment`
- `gitea.issue.close`
- `gitea.pr.close`
- `gitea.branch.delete` (merged-branch cleanup only — see below)
Forbidden on reconciler profiles: `gitea.pr.approve`, `gitea.pr.merge`,
`gitea.pr.review`, `gitea.pr.create`, `gitea.branch.push`, and
`gitea.repo.commit`.
### Merged-branch cleanup ownership (`gitea.branch.delete`)
The reconciler is the repository-supported owner of merged-PR source-branch
cleanup: `task_capability_map` maps `cleanup_merged_pr_branch` (and
`reconciliation_cleanup`) to role `reconciler` with permission
`gitea.branch.delete`. Post-merge branch lifecycle is reconciliation work —
it happens after the author, reviewer, and merger roles have completed, and
it must not be reachable from those roles.
Least-privilege constraints:
- `gitea.branch.delete` is granted **only** to reconciler profiles. Author,
reviewer, and merger profiles must never hold it; `gitea_delete_branch`
and `gitea_cleanup_merged_pr_branch` fail closed on any profile without
the permission.
- Even with the permission, reconciler deletion is only supported through the
guarded `gitea_cleanup_merged_pr_branch` path (#514 / #687): the PR must be
merged, the head an ancestor of the target, the branch not protected
(`master`/`main`/`dev`), the branch not a preservation/evidence ref (e.g.
`chore/issue-681-preserve-review-session-wip`), no open PR may still use the
head, and an explicit `CLEANUP MERGED PR <n> BRANCH <branch>` confirmation is
required. Raw `gitea_delete_branch` is **denied** to reconciler even when
`gitea.branch.delete` is present.
- Raw `git branch -d` / `git push --delete` cleanup remains blocked by
`branch_cleanup_guard` and the final-report validator regardless of
profile permissions.
- `gitea.branch.delete` has no short alias in `GITEA_OPERATION_ALIASES`;
write it fully qualified in `allowed_operations`. Migration must emit
canonical names such as `gitea.pr.close` (never bare `pr.close` /
`issue.close`, which the production normalizer rejects or drops).
Launch a static `gitea-reconciler` MCP namespace with
`GITEA_MCP_PROFILE=prgs-reconciler`. Profile shape is validated by
`reconciler_profile.assess_reconciler_profile` (#304). Use the
@@ -283,159 +251,6 @@ Launch a static `gitea-reconciler` MCP namespace with
fresh target-branch fetch, recorded target SHA, and ancestor proof. PRs whose
heads are not already landed cannot be closed through this path.
### Operational runbook: grant reconciler `gitea.branch.delete` (#687)
Merging a code PR that updates `migrate_profiles.py` / `reconciler_profile.py`
**does not** change the live operator profile on disk. Apply the profile
change deliberately, then reconnect the client-managed namespace.
1. **Approved migration / profile-update command** (from the repo root, using
the project venv if present):
```bash
# Dry-run first (default): validates v2 output, writes nothing
python3 migrate_profiles.py -i ~/.config/gitea-tools/profiles.json
# Apply: creates backup then writes migrated v2 config
python3 migrate_profiles.py -i ~/.config/gitea-tools/profiles.json -w
# Optional explicit paths:
# python3 migrate_profiles.py -i ~/.config/gitea-tools/profiles.json \
# -o ~/.config/gitea-tools/profiles.json \
# --backup ~/.config/gitea-tools/profiles.json.bak -w
```
If the live file is already v2, edit the reconciler identitys
`allowed_operations` / `forbidden_operations` under
`environments.<env>.services.gitea.identities.reconciler` (or the
`prgs-reconciler` alias target) so allowed includes the canonical set
below — then re-validate with a load of the config (see step 3).
2. **Inspect the generated (or edited) profile** — confirm the reconciler
identity, for example:
```bash
python3 - <<'PY'
import json
from pathlib import Path
cfg = json.loads(Path.home().joinpath(".config/gitea-tools/profiles.json").read_text())
# v2 environments shape:
ident = cfg["environments"]["prgs"]["services"]["gitea"]["identities"]["reconciler"]
print("role:", ident.get("role"))
print("allowed:", ident.get("allowed_operations"))
print("forbidden:", ident.get("forbidden_operations"))
PY
```
3. **Validate canonical operation names and least privilege**
Expected canonical **allowed** (defaults after migration):
- `gitea.read`
- `gitea.pr.close` (required)
- `gitea.pr.comment`
- `gitea.issue.comment`
- `gitea.issue.close`
- `gitea.branch.delete` (recommended; cleanup only)
Expected **forbidden** includes at least: `gitea.pr.approve`,
`gitea.pr.merge`, `gitea.pr.review`, `gitea.pr.create`,
`gitea.branch.push`, `gitea.repo.commit`.
No shorthand (`pr.close`, `issue.close`, `pr.comment`) may remain.
Validate with the production loader:
```bash
python3 - <<'PY'
import gitea_config, reconciler_profile
from pathlib import Path
path = str(Path.home() / ".config/gitea-tools/profiles.json")
gitea_config.load_config(path) # fails closed on invalid config
# Or assess the reconciler lists directly after extracting them:
# print(reconciler_profile.assess_reconciler_profile(allowed, forbidden))
PY
```
4. **Merging PR #688 (or any code PR) does not update the live profile.**
Code changes only the migration helper, schema, docs, and tests. The
operator must still run `migrate_profiles.py -w` or an equivalent
authorized edit of `~/.config/gitea-tools/profiles.json`.
5. **Supported apply method:** `python3 migrate_profiles.py … -w` (backup
created automatically) **or** operator-authorized edit of the live
profiles file after backup. Unsupported: silent mtime tricks, manual
process kill to “reload”, or undocumented env overrides.
6. **Backup and validation:** `-w` copies the input to
`<input_path>.bak` (or `--backup PATH`) before writing. Re-run
`load_config` / `assess_reconciler_profile` after write. Keep the
`.bak` until live whoami/capability checks pass.
7. **Client-managed namespace reconnect/reload:** reconnect or reload the
IDE MCP client so `gitea-reconciler` restarts from current `master` and
the updated `GITEA_MCP_PROFILE=prgs-reconciler` config. Do not hand-launch
`mcp_server.py` / `gitea_mcp_server.py` with ad hoc `GITEA_*` env
(see #686 / #630).
8. **Live reverification** (through the client-managed `gitea-reconciler`
namespace only):
- `gitea_whoami` → identity + profile `prgs-reconciler`
- `gitea_assess_master_parity` → `stale=false`, `restart_required=false`
- `gitea_resolve_task_capability(task="cleanup_merged_pr_branch")` →
`allowed_in_current_session=true` only when permission and role match
- `gitea_resolve_task_capability(task="delete_branch")` →
**not** allowed for reconciler (role denial must be enforced)
9. **Guarded cleanup usage** (example for a merged PR whose source branch
remains on the remote):
```text
gitea_cleanup_merged_pr_branch(
pr_number=<N>,
branch=<exact PR head branch>,
confirmation="CLEANUP MERGED PR <N> BRANCH <exact PR head branch>",
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
worktree_path="<path under branches/>",
)
```
The tool refuses unmerged PRs, protected branches, preservation/evidence
branches, open-PR heads, mismatched branch names, and wrong confirmation.
10. **Prohibitions**
- No raw `git push --delete`, `git branch -d` / `-D`, or delete refspecs
- No arbitrary `gitea_delete_branch` from reconciler
- No unsupported profile switching mid-run without full re-preflight
- No ad hoc hand-edits of live profiles **unless** operator-authorized,
backed up, and revalidated as above
Canonical migrated reconciler example:
```json
{
"role": "reconciler",
"allowed_operations": [
"gitea.read",
"gitea.pr.close",
"gitea.pr.comment",
"gitea.issue.comment",
"gitea.issue.close",
"gitea.branch.delete"
],
"forbidden_operations": [
"gitea.pr.approve",
"gitea.pr.merge",
"gitea.pr.review",
"gitea.pr.create",
"gitea.branch.push",
"gitea.repo.commit"
]
}
```
## Identity and fail-closed rules
Before **any** mutating action, a workflow must know both:
+8 -13
View File
@@ -195,7 +195,7 @@ To avoid the bottleneck of relaunching/restarting the MCP server to switch betwe
`gitea_reconcile_already_landed_pr` after ancestry proof — not for normal
review or author workflows.
* **Fallback (operator-owned):** If the dual-profile MCP launcher pattern is not supported or configured in the client, **do not** have the LLM relaunch or restart the client/MCP. The LLM **stops** role-switching work, reports that the correct static namespace is missing, and waits for an **operator** to configure dual namespaces or reload the client with the correct `GITEA_MCP_PROFILE` for that role. Process restart/relaunch is operator-owned under the [stable control runtime ADR](architecture/mcp-stable-control-runtime-policy-adr.md) (#615).
* **Fallback:** If the dual-profile MCP launcher pattern is not supported or configured in the client, the LLM must relaunch or restart the client/MCP with the correct profile environment variable before claiming or working on any tasks.
## Setup runbook — interactive menu
@@ -1200,13 +1200,10 @@ When a mutation blocks on workspace binding:
1. Read the error — it names the **resolved workspace path**, **role
namespace**, and **binding source** (tool arg, env var, or process root).
2. **LLM-allowed:** pass `worktree_path` on reviewer/merger mutation tools when
the active `branches/` worktree differs from the MCP process root; **client
reconnect** after transport EOF only (no process kill).
3. **Operator-owned:** if the wrong namespace process was launched, or a role-
specific `GITEA_*_WORKTREE` must be set at process start, an **operator**
reloads/relaunches the correct static namespace MCP. LLM sessions must not
kill or restart MCP processes (see [stable control runtime ADR](architecture/mcp-stable-control-runtime-policy-adr.md)).
2. Reconnect or relaunch the correct namespace MCP server from the intended
workspace (or set the role-specific env var before launch).
3. Pass `worktree_path` on reviewer/merger mutation tools when the active
branches/ worktree differs from the MCP process root.
4. **Do not** clean, reset, or discard foreign role worktrees to unblock your
own namespace — that destroys another agent's WIP.
@@ -1216,10 +1213,9 @@ When posting a Canonical Thread Handoff after a binding blocker:
- State which namespace was active (author / reviewer / merger / reconciler).
- Quote the resolved workspace path and binding source from the error.
- Name the safe next action: pass `worktree_path` if that unblocks the tool, or
request an **operator** reload of the correct namespace MCP / env binding.
- Explicitly note that foreign worktrees must not be cleaned to unblock, and
that the LLM must not self-restart the MCP process.
- Name the safe reconnect action (relaunch MCP from `branches/...`, set
`GITEA_*_WORKTREE`, or pass `worktree_path`).
- Explicitly note that foreign worktrees must not be cleaned to unblock.
## Safety notes
@@ -1230,7 +1226,6 @@ When posting a Canonical Thread Handoff after a binding blocker:
## Related documents
- [`architecture/mcp-stable-control-runtime-policy-adr.md`](architecture/mcp-stable-control-runtime-policy-adr.md) — stable control runtime vs dev runtime; LLM must not kill/restart MCP; operator-owned reload and promotions; routine post-merge parity staleness (#615).
- [`reviewer-handoff-consistency.md`](reviewer-handoff-consistency.md) — reject contradictory reviewer handoffs (#501).
- [`issue-acceptance-gate.md`](issue-acceptance-gate.md) — controller issue-acceptance audit after PR merge (#500).
- [`../skills/llm-project-workflow/SKILL.md`](../skills/llm-project-workflow/SKILL.md) — portable cross-project LLM workflow skill.
+8 -23
View File
@@ -9,37 +9,22 @@ formal reviews then looked identical to native approvals.
## Rule
Mutation auth, keychain fill, and controller quarantine require a **production
native MCP transport runtime** established only by:
1. the **resolved absolute path** of the canonical entrypoint
(`mcp_server.py` / `gitea_mcp_server.py` next to `mcp_daemon_guard.py`), and
2. a live **transport bind** (`bind_native_mcp_transport(transport="stdio")`)
immediately before `mcp.run`.
Basename-only trust (a renamed file called `mcp_server.py`), caller-controlled
flags (there is **no** `allow_test_bootstrap`), environment variables, stack
frame spoofing, or import-only launch are insufficient.
Mutation auth, keychain fill, and controller quarantine require a **native MCP
transport runtime** established only by the official entrypoint.
| Context | Allowed |
|---------|---------|
| Official IDE-native MCP: resolved canonical entrypoint marks + binds stdio, holds process-local runtime token | yes |
| pytest (hermetic unit tests) via `is_pytest_runtime()` | yes for unit gates |
| `install_test_native_runtime()` under pytest (test-mode record) | unit-test transport gates only — **never** production Gitea mutations |
| `allow_test_bootstrap=True` (removed; must not exist) | **no** |
| Renamed runner basename `mcp_server.py` outside package root | **no** |
| Import/launch of real entrypoint without transport bind | **no** |
| Official MCP entrypoint (`mcp_server.py` / `gitea_mcp_server` `__main__`) calls `mark_sanctioned_daemon()` and holds a process-local runtime token | yes |
| pytest (hermetic unit tests) | yes |
| `GITEA_MCP_SANCTIONED_DAEMON=1` alone (no process-local native runtime) | **no** (#695) |
| `GITEA_ALLOW_DIRECT_MCP_IMPORT=1` in LLM sessions | **no** — never set; never authorizes mutations (#695 AC1 / PR #701) |
| Override `GITEA_MCP_SESSION_STATE_DIR` mid-session | **no** — production bind pins state root; redirect cannot forge independent decision locks (#695 AC2 / PR #701) |
| `GITEA_ALLOW_DIRECT_MCP_IMPORT=1` in LLM sessions | **no** — never set in agent sessions |
| `GITEA_ALLOW_KEYCHAIN_CLI=1` in LLM sessions | **no** — human operator only |
| bare `python -c 'import gitea_mcp_server; …'` or offline runners | **no** |
| keychain fill outside native/pytest | **no** |
Native runtime is **process-local**: a random token bound to the daemon PID and
transport phase. It is never reconstructed from environment variables,
session-state files, caller-controlled flags, or importing internals in a
fresh Python process.
Native runtime is **process-local**: a random token bound to the daemon PID.
It is never reconstructed from environment variables, session-state files, or
importing internals in a fresh Python process.
## Contaminated review quarantine (#695 AC8)
+1 -3
View File
@@ -14,7 +14,6 @@ Handbook for LLM operators and human developers using the Gitea-Tools MCP server
4. **No self-review / no self-merge** — The authenticated Gitea user must not approve or merge a PR they authored.
5. **Follow the gates** — Prompts express intent; MCP tools enforce safety. Never bypass gates via prompt instructions.
6. **Global LLM Worktree Rule** — Main checkout stays on `master`/`main`/`dev`; all mutations happen under `branches/`. Prove project root, `cwd`, branch, stable main-checkout branch, and session worktree path before editing. No exceptions.
7. **Stable control runtime** — Real Gitea mutations use only the **stable** MCP control runtime. LLM sessions must not kill, restart, or relaunch MCP processes, edit the stable checkout, or use a dev MCP for production mutations. See the policy ADR: [mcp-stable-control-runtime-policy-adr.md](../architecture/mcp-stable-control-runtime-policy-adr.md) (#615).
## Supported Gitea instances
@@ -30,5 +29,4 @@ Always pass `remote` explicitly on tool calls. The server default is `dadeschool
1. `gitea_whoami` — confirm authenticated user and profile.
2. `gitea_get_runtime_context` — allowed/forbidden operations for this session.
3. `gitea_resolve_task_capability` — prove the session may perform the planned task.
4. For reviewer work: dry-run validation (`gitea_dry_run_pr_review`) before live review mutations.
5. Confirm master parity / runtime health when tools report stale or unhealthy control runtime — stop mutations and request an **operator** reload of the stable MCP (see [stable control runtime ADR](../architecture/mcp-stable-control-runtime-policy-adr.md)).
4. For reviewer work: dry-run validation (`gitea_dry_run_pr_review`) before live review mutations.
-11
View File
@@ -12,17 +12,6 @@
4. `gitea_mark_final_review_decision` → approve via `gitea_review_pr`.
5. `gitea_merge_pr` with pinned head SHA and `confirmation="MERGE PR <n>"`.
## Stable MCP control runtime (#615)
Policy ADR: [mcp-stable-control-runtime-policy-adr.md](../architecture/mcp-stable-control-runtime-policy-adr.md).
| Situation | Who acts | What to do |
|-----------|----------|------------|
| Transport EOF / missing tools | LLM | **Client reconnect only** — do not kill PIDs |
| Wrong profile / dual-namespace missing | Operator | Configure or reload the correct static namespace(s) |
| Master parity stale after merge | LLM stops + reports; **operator** reloads stable MCP | Resume only after parity re-verified |
| Promote unpromoted MCP code to stable | Operator / release-manager only | Full §2.4 promotion record |
## Gitea Wiki sync
The Gitea Wiki mirrors `docs/wiki/` (source of truth). After merging wiki changes:
+9 -94
View File
@@ -134,22 +134,16 @@ _TARGET_BRANCH_SHA_RE = re.compile(
r"target branch sha\s*:\s*[0-9a-f]{40}",
re.IGNORECASE,
)
# #698: structured proof is rendered in several equivalent shapes —
# `workflow_hash: abc...`, `workflow_hash=abc...`, or JSON
# `"workflow_hash": "abc..."`. Recognize all of them; demanding one exact
# punctuation style rejects legitimate structured workflow-load proof.
_WORKFLOW_LOAD_HELPER_RE = re.compile(
r"workflow[-_ ]load[-_ ]helper[-_ ]result\s*[:=]",
r"workflow[- ]load helper result\s*:",
re.IGNORECASE,
)
_WORKFLOW_LOAD_HASH_RE = re.compile(
r"workflow[-_ ]load[-_ ]helper[-_ ]result[\s\S]{0,400}?"
r"workflow[_ ]hash\"?\s*[:=]\s*\"?[0-9a-f]{12}",
r"workflow[- ]load helper result[\s\S]{0,400}?workflow[_ ]hash\s*:\s*[0-9a-f]{12}",
re.IGNORECASE,
)
_WORKFLOW_LOAD_BOUNDARY_RE = re.compile(
r"workflow[-_ ]load[-_ ]helper[-_ ]result[\s\S]{0,400}?"
r"boundary[_ ]status\"?\s*[:=]\s*\"?(?:clean|violation)",
r"workflow[- ]load helper result[\s\S]{0,400}?boundary[_ ]status\s*:\s*(?:clean|violation)",
re.IGNORECASE,
)
_WORKFLOW_FILE_VIEW_NARRATIVE_RE = re.compile(
@@ -250,59 +244,6 @@ def _normalize_task_kind(task_kind: str | None) -> str:
return _TASK_KIND_ALIASES.get(raw, raw)
def _iter_action_entries(action_log: list | None) -> list[dict]:
"""Yield only structured (dict) action-log entries (#698).
Callers must never crash on malformed entries (strings, numbers, null)
that reach the validator from LLM-composed or partially parsed logs;
:func:`sanitize_action_log` reports them separately.
"""
return [e for e in (action_log or []) if isinstance(e, dict)]
def sanitize_action_log(
action_log: list | None,
) -> tuple[list[dict], list[dict[str, str]]]:
"""Split an action log into structured entries and sanitized findings (#698).
Malformed entries become clear, sanitized ``warning`` findings — the
offending value's content is never echoed back (only its position and
type), so secrets or garbage in a broken log cannot leak into validation
errors, and validation itself proceeds without secondary exceptions.
"""
if action_log is None:
return [], []
if not isinstance(action_log, (list, tuple)):
return [], [
validator_finding(
"shared.action_log_malformed",
"downgrade",
"Action log",
"action_log is not a list of structured entries "
f"(got {type(action_log).__name__}); it was ignored",
"pass action_log as a list of dict entries",
)
]
entries: list[dict] = []
findings: list[dict[str, str]] = []
for index, entry in enumerate(action_log):
if isinstance(entry, dict):
entries.append(entry)
continue
findings.append(
validator_finding(
"shared.action_log_malformed",
"downgrade",
"Action log",
f"action_log entry {index} is not a structured mapping "
f"(got {type(entry).__name__}); the entry was ignored",
"repair the malformed action_log entry or drop it before "
"revalidating",
)
)
return entries, findings
def validator_finding(
rule_id: str,
severity: str,
@@ -480,7 +421,7 @@ def _rule_shared_canonical_comment_post_claim(
rejected_in_report = bool(_CANONICAL_VALIDATION_REJECTED_RE.search(text))
rejected_in_log = False
if action_log:
for entry in _iter_action_entries(action_log):
for entry in action_log:
validation = entry.get("canonical_comment_validation") or {}
if validation.get("allowed") is False:
rejected_in_log = True
@@ -534,13 +475,9 @@ def _rule_reviewer_vague_mutations_none(
action_log: list[dict] | None = None,
mutations_observed: bool = False,
) -> list[dict[str, str]]:
# #698: infer review mutations only from authoritative evidence — an
# entry proves a mutation only when it affirmatively records
# performed=true and was not gated. Read-only diagnostics and pre-API
# rejections (entries without a performed flag) are not mutations.
performed = any(
e.get("performed") is True and not e.get("gated_rejected")
for e in _iter_action_entries(action_log)
e.get("performed") is not False and not e.get("gated_rejected")
for e in (action_log or [])
)
if not (mutations_observed or performed):
return []
@@ -599,7 +536,7 @@ def _rule_reviewer_git_fetch_readonly(
text = report_text or ""
fetch_observed = any(
_GIT_FETCH_RE.search(str(e.get("command") or e.get("action") or ""))
for e in _iter_action_entries(action_log)
for e in (action_log or [])
) or _GIT_FETCH_RE.search(text)
if not fetch_observed:
return []
@@ -1040,7 +977,7 @@ def _rule_reviewer_target_branch_freshness(
fields = _handoff_fields(text)
fetch_reported = bool(_GIT_FETCH_RE.search(text)) or any(
_GIT_FETCH_RE.search(str(e.get("command") or e.get("action") or ""))
for e in _iter_action_entries(action_log)
for e in (action_log or [])
)
target_sha_reported = bool(_TARGET_BRANCH_SHA_RE.search(text)) or any(
"target branch" in key and "sha" in key and _FULL_SHA_RE.search(value)
@@ -1798,12 +1735,6 @@ def assess_final_report_validator(
checks: dict[str, Any] = {}
findings: list[dict[str, str]] = []
# #698: malformed action_log data must never crash validation with a
# secondary exception; malformed entries surface as sanitized findings.
sanitized_action_log, action_log_findings = sanitize_action_log(action_log)
action_log = sanitized_action_log
findings.extend(action_log_findings)
if normalized_kind == "issue_filing" and issue_filing_lock is not None:
checks["issue_filing"] = assess_issue_filing_final_report(
report_text,
@@ -1832,23 +1763,7 @@ def assess_final_report_validator(
}
for rule in _RULES_BY_TASK.get(normalized_kind, ()):
try:
findings.extend(
_call_rule(rule, report_text, normalized_kind, rule_kwargs)
)
except Exception as exc: # #698: fail closed with a sanitized error
findings.append(
validator_finding(
"shared.validator_rule_error",
"block",
"Validator",
f"validator rule '{getattr(rule, '__name__', 'unknown')}' "
f"failed with {type(exc).__name__} (details withheld; "
"sanitized)",
"file a validator defect with the rule name; do not "
"bypass final-report validation",
)
)
findings.extend(_call_rule(rule, report_text, normalized_kind, rule_kwargs))
grade, blocked, downgraded = _aggregate_grade(findings)
reasons = [f"{f['rule_id']}: {f['reason']}" for f in findings]
+19 -207
View File
@@ -243,192 +243,6 @@ def _redact(text):
return str(text)
# ── Classified client failures (#699) ─────────────────────────────────────────
# Subclasses of RuntimeError preserve existing ``except RuntimeError`` call
# sites. Exception *messages* are fixed constants only — HTTP response bodies,
# Keychain material, and arbitrary exception text are never stored on the
# exception or re-emitted to tool results / daemon logs.
# Fixed messages (must match mcp_tool_error_boundary.FIXED_MESSAGES keys used here).
_MSG_AUTH_INVALID = "Gitea authentication failed: invalid or revoked credentials"
_MSG_AUTH_FAILED = "Gitea authentication failed"
_MSG_AUTHZ_SCOPE = "Gitea authorization failed: insufficient token scope"
_MSG_AUTHZ_DENIED = "Gitea authorization failed: access denied"
_MSG_NETWORK = "Network error contacting Gitea"
_MSG_CONFIG = "Gitea configuration or credential resolution failed"
_MSG_UPSTREAM = "Gitea upstream unavailable"
_MSG_HTTP = "Gitea HTTP request failed"
class GiteaClientError(RuntimeError):
"""Base for known Gitea client failures with stable reason_code metadata."""
reason_code = "client_error"
error_class = "client"
http_status = None
def __init__(self, message=None, *, reason_code=None, http_status=None):
if reason_code is not None:
self.reason_code = reason_code
if http_status is not None:
self.http_status = http_status
# Message is always a fixed constant; callers cannot inject bodies.
fixed = message if message is not None else _MSG_HTTP
super().__init__(fixed)
class GiteaAuthError(GiteaClientError):
"""Authentication failure (invalid/revoked credentials → typically HTTP 401)."""
reason_code = "auth_invalid_token"
error_class = "authentication"
http_status = 401
def __init__(self, message=None, *, reason_code=None, http_status=None):
super().__init__(
message if message is not None else _MSG_AUTH_INVALID,
reason_code=reason_code or "auth_invalid_token",
http_status=http_status if http_status is not None else 401,
)
class GiteaAuthzError(GiteaClientError):
"""Authorization failure (HTTP 403 — scope deficiency or access denied)."""
reason_code = "authz_denied"
error_class = "authorization"
http_status = 403
def __init__(self, message=None, *, reason_code=None, http_status=None):
code = reason_code or "authz_denied"
if code == "authz_insufficient_scope":
fixed = _MSG_AUTHZ_SCOPE
else:
fixed = _MSG_AUTHZ_DENIED
code = "authz_denied"
super().__init__(
message if message is not None else fixed,
reason_code=code,
http_status=http_status if http_status is not None else 403,
)
class GiteaNetworkError(GiteaClientError):
"""Transport / DNS / timeout failure contacting Gitea."""
reason_code = "network_error"
error_class = "network"
http_status = None
def __init__(self, message=None, *, reason_code=None, http_status=None):
super().__init__(
message if message is not None else _MSG_NETWORK,
reason_code=reason_code or "network_error",
http_status=http_status,
)
class GiteaConfigError(GiteaClientError):
"""Local configuration / credential resolution failure (not HTTP auth)."""
reason_code = "config_error"
error_class = "configuration"
http_status = None
def __init__(self, message=None, *, reason_code=None, http_status=None):
super().__init__(
message if message is not None else _MSG_CONFIG,
reason_code=reason_code or "config_error",
http_status=http_status,
)
class GiteaHttpError(GiteaClientError):
"""Non-auth HTTP failure with fixed message (no response body)."""
reason_code = "http_error"
error_class = "client"
http_status = None
def __init__(self, message=None, *, reason_code=None, http_status=None):
code = reason_code or "http_error"
if code == "upstream_unavailable":
fixed = _MSG_UPSTREAM
else:
fixed = _MSG_HTTP
code = "http_error"
super().__init__(
message if message is not None else fixed,
reason_code=code,
http_status=http_status,
)
def _looks_like_insufficient_scope(detail: str) -> bool:
"""Internal: inspect redacted body *only* to refine 403 reason_code.
The body is never stored on the exception or returned to callers.
"""
lower = (detail or "").lower()
markers = (
"insufficient scope",
"required scope",
"does not have at least one of required scope",
"token does not have",
"missing scope",
"scope(s)",
)
return any(m in lower for m in markers)
def classify_http_status(code: int, *, body_hint: str = "") -> tuple[type, str, int]:
"""Central HTTP status → (exception_class, reason_code, http_status).
Every HTTP 403 becomes authorization-class. Body text is used only as a
local hint for scope vs denied reason_code and is never returned.
"""
if code == 401:
return (GiteaAuthError, "auth_invalid_token", 401)
if code == 403:
if _looks_like_insufficient_scope(body_hint or ""):
return (GiteaAuthzError, "authz_insufficient_scope", 403)
return (GiteaAuthzError, "authz_denied", 403)
if code in (502, 503, 504):
return (GiteaHttpError, "upstream_unavailable", code)
return (GiteaHttpError, "http_error", code)
def raise_for_http_status(code: int, body: str = "") -> None:
"""Raise a typed client error for *code* without embedding *body*.
*body* may be inspected only to choose scope vs denied for 403; it is
never placed on the exception message.
"""
# Redact before any inspection; discard after classification.
try:
hint = _redact(body or "").strip()
except Exception:
hint = ""
exc_cls, reason, status = classify_http_status(code, body_hint=hint)
# Explicitly construct without passing body/hint into message.
if exc_cls is GiteaAuthError:
raise GiteaAuthError(reason_code=reason, http_status=status)
if exc_cls is GiteaAuthzError:
raise GiteaAuthzError(reason_code=reason, http_status=status)
if reason == "upstream_unavailable":
raise GiteaHttpError(
reason_code="upstream_unavailable",
http_status=status,
)
raise GiteaHttpError(reason_code="http_error", http_status=status)
def _raise_http_error(code: int, detail: str = "") -> None:
"""Backward-compatible alias — *detail* is never embedded in the error."""
raise_for_http_status(code, detail)
def _add_query(url, **params):
"""Return *url* with the given query parameters added or overridden.
@@ -501,24 +315,23 @@ def api_request(method, url, auth_header, payload=None, *,
"""Make an authenticated JSON request to the Gitea API.
Returns parsed JSON on success (or ``None`` for an empty body), and raises
a classified client error on failure.
``RuntimeError`` on failure.
On HTTP 429 the request is retried up to *max_retries* times: honoring a
valid ``Retry-After`` header (seconds or HTTP-date) when present, otherwise
using capped jittered exponential backoff. Successful responses are
unchanged.
Failures raise typed exceptions with **fixed messages only** (#699). HTTP
response bodies are read solely for local 403 reason refinement and are
never stored on exceptions or returned to callers:
All failures are converted to a ``RuntimeError`` with a clear, secret
-redacted message (no raw stack traces or credential material):
- HTTP 401 → :class:`GiteaAuthError` (``auth_invalid_token``)
- HTTP 403 → :class:`GiteaAuthzError` (scope or denied)
- 502/503/504 → :class:`GiteaHttpError` (``upstream_unavailable``)
- Other non-429 HTTP → :class:`GiteaHttpError` (``http_error``)
- Timeouts / DNS / ``URLError`` → :class:`GiteaNetworkError`
- Malformed success JSON → plain ``RuntimeError`` (programming/protocol;
not reclassified as authentication)
- Non-429 HTTP errors surface the status code and a redacted response body.
502/503/504 upstream errors get an explicit "Gitea upstream unavailable"
message.
- Timeouts and network/DNS failures (``URLError`` / ``TimeoutError``) surface
a generic "network error contacting Gitea" message.
- A malformed (non-JSON) success body surfaces a "malformed JSON response"
message rather than a raw decode error.
The ``*_func`` parameters and ``timeout`` are injection points for
deterministic testing.
@@ -556,23 +369,22 @@ def api_request(method, url, auth_header, payload=None, *,
error_body = e.read().decode("utf-8", errors="replace")
except Exception:
error_body = ""
# Classify from status (+ local body hint). Body is not embedded.
try:
raise_for_http_status(e.code, error_body)
except GiteaClientError:
raise
# Defensive: raise_for_http_status always raises.
raise GiteaHttpError(http_status=e.code) from e # pragma: no cover
detail = _redact(error_body).strip()
if e.code in (502, 503, 504):
msg = f"HTTP {e.code}: Gitea upstream unavailable"
raise RuntimeError(f"{msg}: {detail}" if detail else msg) from e
raise RuntimeError(f"HTTP {e.code}: {detail}") from e
except (urllib.error.URLError, TimeoutError) as e:
# Fixed message only — do not embed URLError reason (may leak paths).
raise GiteaNetworkError(reason_code="network_error") from e
reason = getattr(e, "reason", e)
raise RuntimeError(
f"network error contacting Gitea: {_redact(reason)}"
) from e
if not body:
return None
try:
return json.loads(body)
except ValueError as e:
# Programming/protocol failure — not authentication.
raise RuntimeError("malformed JSON response from Gitea") from e
+98 -2394
View File
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
+34 -339
View File
@@ -6,18 +6,12 @@ Direct ``import gitea_mcp_server`` from a shell bypasses native MCP transport.
- Environment variables alone cannot reconstruct a native session
(``GITEA_MCP_SANCTIONED_DAEMON=1`` / ``GITEA_ALLOW_DIRECT_MCP_IMPORT=1`` are
insufficient for mutation gates).
- A process-local runtime record is established only by the resolved canonical
entrypoint path (not basename) **and** the actual native MCP transport
lifecycle (``bind_native_mcp_transport`` before ``mcp.run``). Merely
importing or launching the entrypoint offline does not grant mutation
authority.
- Public caller-controlled flags (including any former
``allow_test_bootstrap``) never establish trusted mutation provenance.
- A process-local runtime record is created only by the official entrypoint
(``mcp_server.py`` calling ``mark_sanctioned_daemon``), bound to PID and a
random secret that never leaves process memory.
- Offline scripts that import internals fail closed on mutations.
- Pytest remains allowed for hermetic unit tests via ``is_pytest_runtime()``.
A separate test-only seam may establish a **test-mode** native record for
unit tests of transport gates; that record cannot authorize production
Gitea mutation endpoints.
- Pytest remains allowed (hermetic tests); optional force flags exist for
provenance regression tests.
Manual deletion of session-state files is never a recovery path.
"""
@@ -29,7 +23,6 @@ import inspect
import os
import secrets
import time
from pathlib import Path
from typing import Any
SANCTIONED_DAEMON_ENV = "GITEA_MCP_SANCTIONED_DAEMON"
@@ -41,18 +34,6 @@ FORCE_PROVENANCE_FAIL_ENV = "GITEA_TEST_FORCE_UNSANCTIONED"
# Process-local native runtime (never persisted, never read from env alone).
_NATIVE_RUNTIME: dict[str, Any] | None = None
# Production transport identifiers accepted by bind_native_mcp_transport.
_PRODUCTION_TRANSPORTS = frozenset({"stdio"})
_RUNTIME_MODE_PRODUCTION = "production"
_RUNTIME_MODE_TEST = "test"
_PHASE_ENTRYPOINT_CLAIMED = "entrypoint_claimed"
_PHASE_TRANSPORT_BOUND = "transport_bound"
# Default session-state root (mirrors mcp_session_state; kept local to avoid
# import cycles). Used only to pin authority at transport bind (#695 AC2).
_DEFAULT_SESSION_STATE_DIR = os.path.expanduser("~/.cache/gitea-tools/session-state")
SESSION_STATE_DIR_ENV = "GITEA_MCP_SESSION_STATE_DIR"
class UnsanctionedRuntimeError(RuntimeError):
"""Raised when mutation/credential code runs outside a native MCP daemon."""
@@ -72,261 +53,52 @@ def is_pytest_runtime() -> bool:
return bool((os.environ.get("PYTEST_CURRENT_TEST") or "").strip())
def _package_root() -> Path:
"""Directory that contains the canonical MCP entrypoint modules."""
return Path(__file__).resolve().parent
def canonical_entrypoint_paths() -> frozenset[str]:
"""Resolved absolute paths of official entrypoints (not basenames)."""
root = _package_root()
return frozenset(
{
str((root / "mcp_server.py").resolve()),
str((root / "gitea_mcp_server.py").resolve()),
}
)
def _resolve_path(path: str | None) -> str | None:
if not path:
return None
try:
return str(Path(path).resolve())
except (OSError, RuntimeError, ValueError):
return None
def _caller_official_entrypoint_path() -> str | None:
"""Return the resolved canonical entrypoint path in the call stack, or None.
Basename-only matches (e.g. an attacker file named ``mcp_server.py``
elsewhere) are rejected. The path must equal one of
:func:`canonical_entrypoint_paths`.
"""
canonical = canonical_entrypoint_paths()
for frame in inspect.stack()[1:20]:
resolved = _resolve_path(frame.filename)
if resolved and resolved in canonical:
return resolved
return None
def _caller_is_official_entrypoint() -> bool:
"""True when invoked from a resolved canonical entrypoint path (#695)."""
return _caller_official_entrypoint_path() is not None
"""True when mark_sanctioned_daemon is invoked from mcp_server.py."""
for frame in inspect.stack()[1:12]:
path = (frame.filename or "").replace("\\", "/")
base = path.rsplit("/", 1)[-1]
if base == "mcp_server.py":
return True
return False
def _new_runtime_token() -> tuple[str, str]:
token = secrets.token_hex(32)
fingerprint = hashlib.sha256(token.encode()).hexdigest()[:16]
return token, fingerprint
def mark_sanctioned_daemon(*, allow_test_bootstrap: bool = False) -> dict[str, Any]:
"""Mark this process as the official native MCP daemon (#695).
def mark_sanctioned_daemon() -> dict[str, Any]:
"""Claim the official entrypoint for this process (#695).
This alone does **not** authorize mutations. Callers must subsequently
bind the native MCP transport via :func:`bind_native_mcp_transport`.
Only a stack frame whose **resolved absolute path** is the canonical
``mcp_server.py`` or ``gitea_mcp_server.py`` next to this module may
claim the entrypoint. Basename spoofing is rejected.
There is no public ``allow_test_bootstrap`` argument: caller-controlled
flags must never establish trusted mutation provenance. Hermetic tests
use :func:`install_test_native_runtime` (pytest-only, test mode).
Only the official ``mcp_server.py`` entrypoint (or pytest test bootstrap)
may establish native transport. Setting env vars alone is insufficient.
"""
global _NATIVE_RUNTIME
if is_pytest_runtime():
# Under pytest, production mark is a no-op for transport authority.
# Tests that need a native-transport record use install_test_native_runtime.
return native_runtime_status()
entrypoint_path = _caller_official_entrypoint_path()
if entrypoint_path is None:
raise UnsanctionedRuntimeError(
"mark_sanctioned_daemon rejected: not called from the resolved "
"canonical MCP entrypoint path (#695). Basename-only names "
"(e.g. a renamed runner called mcp_server.py) are insufficient. "
"Offline import / standalone scripts cannot reconstruct native "
"transport. Stop after native MCP failure; do not run offline "
"mutation helpers."
)
token, fingerprint = _new_runtime_token()
if not is_pytest_runtime() and not allow_test_bootstrap:
if not _caller_is_official_entrypoint():
raise UnsanctionedRuntimeError(
"mark_sanctioned_daemon rejected: not called from official "
"mcp_server.py entrypoint (#695). Offline import / standalone "
"scripts cannot reconstruct native transport. Stop after native "
"MCP failure; do not run offline mutation helpers."
)
token = secrets.token_hex(32)
_NATIVE_RUNTIME = {
"token": token,
"token_fingerprint": fingerprint,
"token_fingerprint": hashlib.sha256(token.encode()).hexdigest()[:16],
"pid": os.getpid(),
"started_at": time.time(),
"entrypoint": "mcp_server",
"entrypoint_path": entrypoint_path,
"phase": _PHASE_ENTRYPOINT_CLAIMED,
"transport": None,
"mode": _RUNTIME_MODE_PRODUCTION,
}
# Legacy signal for older probes; alone does not authorize mutations.
os.environ[SANCTIONED_DAEMON_ENV] = "1"
return native_runtime_status()
def bind_native_mcp_transport(*, transport: str) -> dict[str, Any]:
"""Bind the live native MCP transport lifecycle (#695).
Must be called from the resolved canonical entrypoint immediately before
the real MCP server transport loop (e.g. ``mcp.run(transport=\"stdio\")``).
Requires a prior successful :func:`mark_sanctioned_daemon` claim in this
process. Import-only or offline launch without this bind leaves
:func:`is_native_mcp_transport` false.
"""
global _NATIVE_RUNTIME
transport_name = (transport or "").strip().lower()
if transport_name not in _PRODUCTION_TRANSPORTS:
raise UnsanctionedRuntimeError(
f"bind_native_mcp_transport rejected: transport {transport!r} is "
f"not a production MCP transport (#695). Allowed: "
f"{sorted(_PRODUCTION_TRANSPORTS)}."
)
entrypoint_path = _caller_official_entrypoint_path()
if entrypoint_path is None:
raise UnsanctionedRuntimeError(
"bind_native_mcp_transport rejected: not called from the resolved "
"canonical MCP entrypoint path (#695)."
)
if _NATIVE_RUNTIME is None or int(_NATIVE_RUNTIME.get("pid") or -1) != os.getpid():
raise UnsanctionedRuntimeError(
"bind_native_mcp_transport rejected: no entrypoint claim in this "
"process (#695). Call mark_sanctioned_daemon() from the official "
"entrypoint first."
)
if _NATIVE_RUNTIME.get("mode") != _RUNTIME_MODE_PRODUCTION:
raise UnsanctionedRuntimeError(
"bind_native_mcp_transport rejected: runtime mode is not "
"production (#695)."
)
claimed = (_NATIVE_RUNTIME.get("entrypoint_path") or "").strip()
if claimed and claimed != entrypoint_path:
raise UnsanctionedRuntimeError(
"bind_native_mcp_transport rejected: entrypoint path mismatch "
"between mark and bind (#695)."
)
# Pin session-state root for this server lifetime (#695 AC2 / PR #701).
# Changing GITEA_MCP_SESSION_STATE_DIR after bind must not manufacture a
# second authority domain for decision locks / workflow proofs.
raw_state = (os.environ.get(SESSION_STATE_DIR_ENV) or "").strip()
if not raw_state:
raw_state = _DEFAULT_SESSION_STATE_DIR
try:
pinned_state = str(Path(raw_state).resolve())
except (OSError, RuntimeError, ValueError):
pinned_state = raw_state
_NATIVE_RUNTIME["phase"] = _PHASE_TRANSPORT_BOUND
_NATIVE_RUNTIME["transport"] = transport_name
_NATIVE_RUNTIME["entrypoint_path"] = entrypoint_path
_NATIVE_RUNTIME["bound_at"] = time.time()
_NATIVE_RUNTIME["session_state_dir"] = pinned_state
os.environ[SANCTIONED_DAEMON_ENV] = "1"
return native_runtime_status()
def install_test_native_runtime() -> dict[str, Any]:
"""Pytest-only seam for hermetic native-transport unit tests (#695).
Establishes a **test-mode** process-local record so unit tests can exercise
gates that require ``is_native_mcp_transport()``. This record:
- is rejected outside pytest (including a fresh offline interpreter);
- never uses production mode;
- cannot authorize production Gitea mutation endpoints
(:func:`assert_production_mutation_runtime` / production path of
:func:`assert_sanctioned_mutation_runtime` when not under pytest).
There is no public caller-controlled flag that forges production native
transport.
"""
global _NATIVE_RUNTIME
if not is_pytest_runtime():
raise UnsanctionedRuntimeError(
"install_test_native_runtime rejected: test-mode native runtime "
"is only available under pytest (#695). allow_test_bootstrap and "
"similar caller-controlled flags do not exist and cannot authorize "
"a fresh offline interpreter."
)
token, fingerprint = _new_runtime_token()
_NATIVE_RUNTIME = {
"token": token,
"token_fingerprint": fingerprint,
"pid": os.getpid(),
"started_at": time.time(),
"entrypoint": "test_bootstrap",
"entrypoint_path": None,
"phase": _PHASE_TRANSPORT_BOUND,
"transport": "test",
"mode": _RUNTIME_MODE_TEST,
"bound_at": time.time(),
}
return native_runtime_status()
def clear_native_runtime_for_tests() -> None:
"""Test helper: drop native runtime (does not clear env)."""
global _NATIVE_RUNTIME
_NATIVE_RUNTIME = None
def pinned_session_state_dir() -> str | None:
"""Session-state root pinned for this production transport lifetime (#695 AC2).
When production native transport is bound, durable session proofs must use
this directory only. Env overrides of ``GITEA_MCP_SESSION_STATE_DIR`` after
bind are ignored so redirected dirs (e.g. ``.mcp_session_701``) cannot
manufacture independent decision-lock authority (PR #701 recurrence).
"""
if not is_production_native_mcp_transport():
return None
pinned = (_NATIVE_RUNTIME or {}).get("session_state_dir")
text = (str(pinned) if pinned is not None else "").strip()
return text or None
def direct_import_env_enabled() -> bool:
"""True when the legacy direct-import opt-in env is set (never authorizes)."""
return (os.environ.get(ALLOW_DIRECT_IMPORT_ENV) or "").strip().lower() in {
"1",
"true",
"yes",
}
def assert_no_direct_import_bypass(context: str = "mutation") -> None:
"""Fail closed when GITEA_ALLOW_DIRECT_MCP_IMPORT is used for mutations (#695 AC1).
The env flag is never a sanctioned recovery path for LLM/agent sessions.
Under pytest hermetic tests this is a no-op so unit tests can set the flag
to prove it does not grant authority.
"""
if is_pytest_runtime():
return
if not direct_import_env_enabled():
return
raise UnsanctionedRuntimeError(
f"{ALLOW_DIRECT_IMPORT_ENV} does not authorize {context} (#695 AC1). "
"Direct import of gitea_mcp_server mutation tools is forbidden. "
"Stop after native MCP failure; reconnect the official MCP daemon. "
"Do not set direct-import flags, offline runners, or redirected "
f"{SESSION_STATE_DIR_ENV} directories to reconstruct gates."
)
def is_native_mcp_transport() -> bool:
"""True when this process holds a transport-bound native runtime (#695)."""
"""True when this process holds a live native MCP runtime record (#695)."""
if (os.environ.get(FORCE_PROVENANCE_FAIL_ENV) or "").strip() in {
"1",
"true",
@@ -339,23 +111,12 @@ def is_native_mcp_transport() -> bool:
return False
if not (_NATIVE_RUNTIME.get("token") or "").strip():
return False
if _NATIVE_RUNTIME.get("phase") != _PHASE_TRANSPORT_BOUND:
return False
if not (_NATIVE_RUNTIME.get("transport") or "").strip():
return False
return True
def is_production_native_mcp_transport() -> bool:
"""True only for production-mode, transport-bound native runtime."""
if not is_native_mcp_transport():
return False
return (_NATIVE_RUNTIME or {}).get("mode") == _RUNTIME_MODE_PRODUCTION
def is_sanctioned_mcp_daemon() -> bool:
"""Backward-compatible name; #695 requires native transport, not env alone."""
if is_production_native_mcp_transport():
if is_native_mcp_transport():
return True
if is_pytest_runtime():
return True
@@ -365,45 +126,10 @@ def is_sanctioned_mcp_daemon() -> bool:
return False
def assert_production_mutation_runtime(context: str = "mutation") -> None:
"""Fail closed unless production native MCP transport is bound (#695).
Test-mode bootstrap records and pytest-only hermetic allowances do **not**
satisfy this gate. Use for production Gitea mutation endpoints that must
never be reachable via test bootstrap.
"""
if is_production_native_mcp_transport():
return
mode = (_NATIVE_RUNTIME or {}).get("mode")
if mode == _RUNTIME_MODE_TEST:
raise UnsanctionedRuntimeError(
f"Test-mode native runtime cannot authorize production {context} "
"(#695). install_test_native_runtime / former allow_test_bootstrap "
"must never reach real Gitea mutation endpoints."
)
assert_sanctioned_mutation_runtime(context)
def assert_sanctioned_mutation_runtime(context: str = "mutation") -> None:
"""Fail closed when mutation code runs outside native MCP transport (#695).
Under pytest, hermetic unit tests are allowed (profile/permission tests).
Outside pytest, requires production-mode transport-bound native runtime.
Test-mode records do not authorize non-pytest production mutations.
``GITEA_ALLOW_DIRECT_MCP_IMPORT`` never authorizes mutations (#695 AC1).
"""
if is_pytest_runtime():
"""Fail closed when mutation code runs outside native MCP transport (#695)."""
if is_sanctioned_mcp_daemon():
return
# AC1: direct-import env is never a mutation recovery path (PR #701).
assert_no_direct_import_bypass(context)
if is_production_native_mcp_transport():
return
mode = (_NATIVE_RUNTIME or {}).get("mode")
if mode == _RUNTIME_MODE_TEST:
raise UnsanctionedRuntimeError(
f"Test-mode native runtime cannot authorize production {context} "
"(#695). Test bootstrap cannot reach production mutation endpoints."
)
env_spoof = (os.environ.get(SANCTIONED_DAEMON_ENV) or "").strip() in {
"1",
"true",
@@ -413,26 +139,14 @@ def assert_sanctioned_mutation_runtime(context: str = "mutation") -> None:
if env_spoof:
extra = (
f" Note: {SANCTIONED_DAEMON_ENV} alone is not sufficient (#695); "
"native transport requires the official MCP entrypoint and a live "
"transport bind."
)
phase = (_NATIVE_RUNTIME or {}).get("phase")
if phase == _PHASE_ENTRYPOINT_CLAIMED:
extra = (
(extra + " ") if extra else " "
) + (
"Entrypoint was claimed but native MCP transport was never bound "
"(#695); offline launch/import of the real entrypoint does not "
"grant mutation authority."
"native transport requires the official MCP entrypoint."
)
raise UnsanctionedRuntimeError(
f"Unsanctioned / non-native runtime blocked {context} (#695). "
"Do not import gitea_mcp_server or call mutation helpers from a raw "
"shell, offline runner, or ad-hoc script after native MCP failure. "
"Stop and reconnect the official MCP daemon (mcp_server.py) over "
"native transport. "
f"Do not set {ALLOW_DIRECT_IMPORT_ENV}, override "
f"{SESSION_STATE_DIR_ENV}, or use raw token env vars in LLM "
"Stop and reconnect the official MCP daemon (mcp_server.py). "
f"Do not set {ALLOW_DIRECT_IMPORT_ENV} or raw token env vars in LLM "
f"sessions.{extra}"
)
@@ -467,23 +181,14 @@ def native_runtime_status() -> dict[str, Any]:
rt = _NATIVE_RUNTIME or {}
return {
"native_mcp_transport": is_native_mcp_transport(),
"production_native_mcp_transport": is_production_native_mcp_transport(),
"pytest": is_pytest_runtime(),
"pid": rt.get("pid"),
"token_fingerprint": rt.get("token_fingerprint"),
"started_at": rt.get("started_at"),
"entrypoint": rt.get("entrypoint"),
"entrypoint_path": rt.get("entrypoint_path"),
"phase": rt.get("phase"),
"transport": rt.get("transport"),
"mode": rt.get("mode"),
"session_state_dir": pinned_session_state_dir() or rt.get("session_state_dir"),
"session_state_dir_pinned": pinned_session_state_dir() is not None,
"direct_import_env_set": direct_import_env_enabled(),
"env_sanctioned_alone_insufficient": True,
"sanctioned_env": SANCTIONED_DAEMON_ENV,
"allow_direct_import_env": ALLOW_DIRECT_IMPORT_ENV,
"session_state_dir_env": SESSION_STATE_DIR_ENV,
"allow_keychain_cli_env": ALLOW_KEYCHAIN_CLI_ENV,
}
@@ -498,20 +203,10 @@ def runtime_status() -> dict[str, Any]:
def mutation_provenance_fields() -> dict[str, Any]:
"""Fields to attach to live mutation / review audit records (#695 AC6)."""
st = native_runtime_status()
transport = "native_mcp" if st["native_mcp_transport"] else "untrusted"
if st.get("mode") == _RUNTIME_MODE_TEST and st["native_mcp_transport"]:
transport = "test_native_mcp"
return {
"transport": transport,
"transport": "native_mcp" if st["native_mcp_transport"] else "untrusted",
"native_mcp_transport": bool(st["native_mcp_transport"]),
"production_native_mcp_transport": bool(
st.get("production_native_mcp_transport")
),
"native_runtime_pid": st.get("pid"),
"native_token_fingerprint": st.get("token_fingerprint"),
"entrypoint": st.get("entrypoint"),
"phase": st.get("phase"),
"mode": st.get("mode"),
"session_state_dir": st.get("session_state_dir"),
"session_state_dir_pinned": bool(st.get("session_state_dir_pinned")),
}
+3 -5
View File
@@ -41,17 +41,15 @@ def check_conflict_markers():
check_conflict_markers()
# #558 / #695: claim the official entrypoint before loading mutation modules.
# This alone does NOT authorize mutations — gitea_mcp_server binds the live
# native MCP transport (stdio) immediately before mcp.run. Import-only or
# offline launch without that bind fails closed on mutations.
# #558: official entrypoint marks the process as the sanctioned MCP daemon
# before loading mutation modules (blocks raw shell import bypasses).
try:
import mcp_daemon_guard
mcp_daemon_guard.mark_sanctioned_daemon()
except Exception:
# Guard import failures must not hide conflict-marker infra_stop above;
# gitea_mcp_server main also marks + binds when run as __main__.
# gitea_mcp_server main also marks sanctioned when run as __main__.
pass
# Execute the actual server logic via exec in this namespace.
+1 -368
View File
@@ -36,31 +36,7 @@ KIND_REVIEW_DRAFT = "review_draft"
# other session proofs; a contaminated session fails closed on gated mutations
# until a reconciler audits and clears it.
KIND_STABLE_BRANCH_CONTAMINATION = "stable_branch_contamination"
# #709: archive prior terminal decision ledgers instead of silent overwrite.
KIND_DECISION_LOCK_ARCHIVE = "review_decision_lock_archive"
# #709: post-merge cleanup/audit reconciliation-required durable record.
KIND_POST_MERGE_DECISION_RECOVERY = "post_merge_decision_recovery"
# #709: truthful record when historical terminal evidence is irrecoverably gone.
KIND_IRRECOVERABLE_DECISION_PROVENANCE = "irrecoverable_decision_provenance"
# #709 F1: server-side non-forgeable authorization artifact for recovery.
KIND_IRRECOVERABLE_PROVENANCE_AUTH = "irrecoverable_provenance_authorization"
# Kinds that must survive the default session-state TTL (forensic / recovery).
#
# KIND_DECISION_LOCK is recovery-critical (#720): terminal review provenance is
# not disposable cache. A generic four-hour TTL must not drop old-head evidence
# or make ``fresh_review_on_current_head_allowed`` unreachable. Same-head / same-
# run #332 protections still apply once the ledger is loadable. Other session
# kinds (workflow load, drafts, etc.) remain TTL-bound.
RECOVERY_CRITICAL_KINDS = frozenset(
{
KIND_DECISION_LOCK,
KIND_DECISION_LOCK_ARCHIVE,
KIND_POST_MERGE_DECISION_RECOVERY,
KIND_IRRECOVERABLE_DECISION_PROVENANCE,
KIND_IRRECOVERABLE_PROVENANCE_AUTH,
}
)
_SAFE_SEGMENT_RE = re.compile(r"[^A-Za-z0-9._+-]+")
@@ -68,34 +44,6 @@ SESSION_PROFILE_LOCK_ENV = "GITEA_SESSION_PROFILE_LOCK"
def default_state_dir() -> str:
"""Resolve the durable session-state root.
When production native MCP transport is bound (#695 AC2), the directory
pinned at transport bind is authoritative: later overrides of
``GITEA_MCP_SESSION_STATE_DIR`` cannot manufacture a second authority
domain (PR #701 recurrence: ``.mcp_session_701`` evasion of cross-PR
decision locks).
"""
try:
import mcp_daemon_guard
pinned = mcp_daemon_guard.pinned_session_state_dir()
if pinned:
return pinned
except Exception:
# Fail open to env/default only when guard is unavailable (e.g. partial
# import during bootstrap). Mutation gates still fail closed separately.
pass
raw = (os.environ.get(STATE_DIR_ENV) or DEFAULT_STATE_DIR).strip()
return raw or DEFAULT_STATE_DIR
def env_session_state_dir_unpinned() -> str:
"""Raw env/default session-state dir ignoring production transport pin.
Intended for diagnostics and tests that assert pin behavior — not for
mutation-sensitive durable proofs under a bound native daemon.
"""
raw = (os.environ.get(STATE_DIR_ENV) or DEFAULT_STATE_DIR).strip()
return raw or DEFAULT_STATE_DIR
@@ -252,16 +200,6 @@ def _write_json(path: str, data: dict[str, Any]) -> None:
pass
def is_recovery_critical_record(record: dict[str, Any] | None, kind: str | None = None) -> bool:
"""True when a durable record must outlive the generic session-state TTL."""
if not record and not kind:
return False
record_kind = ((record or {}).get("kind") or kind or "").strip()
if record_kind in RECOVERY_CRITICAL_KINDS:
return True
return bool((record or {}).get("recovery_critical"))
def identity_match_reasons(
record: dict[str, Any] | None,
*,
@@ -304,9 +242,7 @@ def identity_match_reasons(
reasons.append("session state missing recorded_at timestamp (fail closed)")
else:
age = _now_utc() - recorded_at
kind = (record.get("kind") or "").strip()
ttl_exempt = is_recovery_critical_record(record, kind=kind)
if age > timedelta(hours=ttl_hours()) and not ttl_exempt:
if age > timedelta(hours=ttl_hours()):
reasons.append(
f"session state expired after {ttl_hours():g}h (fail closed)"
)
@@ -315,113 +251,6 @@ def identity_match_reasons(
return reasons
def inspect_state_envelope(
*,
kind: str,
remote: str | None = None,
org: str | None = None,
repo: str | None = None,
profile_identity: str | None = None,
state_dir: str | None = None,
) -> dict[str, Any]:
"""Read-only disk inspection for assessment when TTL would otherwise hide state (#720).
Does **not** apply identity/TTL rejection to the returned presence flags.
Callers use this to distinguish:
* no file on disk
* file present but TTL would reject a non-critical kind
* recovery-critical ledger (e.g. KIND_DECISION_LOCK) still loadable
Never mutates files. Never returns secrets.
"""
profile = current_profile_identity(profile_identity=profile_identity)
path = state_file_path(
kind=kind,
remote=remote,
org=org,
repo=repo,
profile_identity=profile,
state_dir=state_dir,
)
result: dict[str, Any] = {
"kind": kind,
"profile_identity": profile,
"path_basename": os.path.basename(path),
"on_disk": False,
"has_payload": False,
"recorded_at": None,
"updated_at": None,
"age_hours": None,
"ttl_hours": ttl_hours(),
"age_exceeds_default_ttl": False,
"recovery_critical": kind in RECOVERY_CRITICAL_KINDS,
"ttl_exempt": kind in RECOVERY_CRITICAL_KINDS,
"would_ttl_reject": False,
"identity_reasons": [],
"summary": "no session-state file on disk",
}
if not path or not os.path.exists(path):
return result
result["on_disk"] = True
envelope = _read_json(path)
if not envelope:
result["summary"] = "session-state file present but unreadable or empty"
return result
payload = envelope.get("payload")
merged: dict[str, Any] = dict(payload) if isinstance(payload, dict) else {}
result["has_payload"] = isinstance(payload, dict)
for key in (
"kind",
"remote",
"org",
"repo",
"profile_identity",
"session_profile_lock",
"recorded_at",
"updated_at",
"writer_pid",
"recovery_critical",
):
if key in envelope and key not in merged:
merged[key] = envelope[key]
if not merged.get("kind"):
merged["kind"] = kind
recorded_at = _parse_iso(merged.get("recorded_at") or merged.get("updated_at"))
result["recorded_at"] = merged.get("recorded_at") or merged.get("updated_at")
result["updated_at"] = merged.get("updated_at") or merged.get("recorded_at")
if recorded_at is not None:
age = _now_utc() - recorded_at
age_hours = age.total_seconds() / 3600.0
result["age_hours"] = age_hours
result["age_exceeds_default_ttl"] = age > timedelta(hours=ttl_hours())
ttl_exempt = is_recovery_critical_record(merged, kind=kind)
result["recovery_critical"] = ttl_exempt
result["ttl_exempt"] = ttl_exempt
identity_reasons = identity_match_reasons(
merged,
remote=remote,
org=org,
repo=repo,
profile_identity=profile,
)
result["identity_reasons"] = list(identity_reasons)
result["would_ttl_reject"] = any("expired" in r for r in identity_reasons)
if result["has_payload"] and ttl_exempt:
result["summary"] = (
"recovery-critical session-state present on disk and TTL-exempt; "
"load via load_state for full payload"
)
elif result["has_payload"] and result["would_ttl_reject"]:
result["summary"] = (
"session-state file present on disk but generic TTL would reject load "
f"(age_hours={result.get('age_hours')!r}, ttl={ttl_hours():g}h)"
)
elif result["has_payload"]:
result["summary"] = "session-state file present and within TTL / identity gates"
else:
result["summary"] = "session-state file present without a dict payload"
return result
def load_state(
*,
kind: str,
@@ -533,26 +362,6 @@ def save_state(
body["org"] = key_org
if key_repo is not None:
body["repo"] = key_repo
# Stamp session-state authority used for this write (#695 AC2 / AC6).
body.setdefault("session_state_dir", root)
try:
import mcp_daemon_guard
prov = mcp_daemon_guard.mutation_provenance_fields()
body.setdefault(
"native_token_fingerprint", prov.get("native_token_fingerprint")
)
body.setdefault(
"native_mcp_transport", bool(prov.get("native_mcp_transport"))
)
body.setdefault(
"production_native_mcp_transport",
bool(prov.get("production_native_mcp_transport")),
)
body.setdefault("transport", prov.get("transport"))
except Exception:
body.setdefault("native_mcp_transport", False)
body.setdefault("transport", "untrusted")
envelope = {
"kind": kind,
@@ -564,8 +373,6 @@ def save_state(
"recorded_at": body["recorded_at"],
"updated_at": body["updated_at"],
"writer_pid": body["writer_pid"],
"session_state_dir": body.get("session_state_dir"),
"transport": body.get("transport"),
"payload": body,
}
_write_json(path, envelope)
@@ -590,177 +397,3 @@ def clear_state(
profile_identity=profile_identity,
state_dir=state_dir,
)
def list_decision_lock_profile_identities(
state_dir: str | None = None,
) -> list[str]:
"""Return profile identities that have a durable review_decision_lock file (#709).
Filename form: ``review_decision_lock-<profile>.json`` (see ``state_key``).
Does not validate TTL or identity — callers must load via ``load_state``.
"""
root = (state_dir or default_state_dir()).strip()
if not root or not os.path.isdir(root):
return []
prefix = f"{_sanitize_segment(KIND_DECISION_LOCK)}-"
suffix = ".json"
found: list[str] = []
try:
names = os.listdir(root)
except OSError:
return []
for name in names:
if not name.startswith(prefix) or not name.endswith(suffix):
continue
if name.endswith(".lock"):
continue
mid = name[len(prefix) : -len(suffix)]
if mid:
found.append(mid)
return sorted(set(found))
def load_state_for_profile(
*,
kind: str,
profile_identity: str,
remote: str | None = None,
org: str | None = None,
repo: str | None = None,
state_dir: str | None = None,
skip_identity_match: bool = False,
enforce_repo_scope: bool = True,
) -> dict[str, Any] | None:
"""Load durable state for an explicit profile identity (#709 cross-profile).
When *skip_identity_match* is True, still requires the file's recorded
profile_identity to equal the requested profile (anti-stomp), but does not
require the *active* session identity to match — needed so a merger can
inspect a reviewer lock after merge.
#709 F3: remote/org/repo filter reasons are **enforced** (not merely
computed). When *enforce_repo_scope* is True (default) and the caller
supplies remote/org/repo, mismatches fail closed with ``None``.
"""
# #709 F3: refuse traversal / malformed profile identities.
try:
from irrecoverable_provenance import assess_profile_path_identity
path_gate = assess_profile_path_identity(profile_identity)
if not path_gate.get("valid"):
return None
except Exception:
# Module may be mid-import in edge bootstraps; fall through to
# conservative checks below.
raw = str(profile_identity or "")
if ".." in raw or "/" in raw or "\\" in raw or "\x00" in raw:
return None
profile = current_profile_identity(profile_identity=profile_identity)
root = _ensure_state_dir(state_dir)
# Refuse symlink escape of the state root (#709 F3).
try:
real_root = os.path.realpath(root)
path = state_file_path(
kind=kind,
remote=remote,
org=org,
repo=repo,
profile_identity=profile,
state_dir=root,
)
real_path = os.path.realpath(path) if os.path.exists(path) else path
if os.path.exists(path) and not str(real_path).startswith(
str(real_root) + os.sep
) and str(real_path) != str(real_root):
return None
except OSError:
return None
path = state_file_path(
kind=kind,
remote=remote,
org=org,
repo=repo,
profile_identity=profile,
state_dir=root,
)
lock_path = f"{path}.lock"
with _exclusive_file_lock(lock_path):
envelope = _read_json(path)
if not envelope:
return None
payload = envelope.get("payload")
if not isinstance(payload, dict):
return None
merged = dict(payload)
for key in (
"kind",
"remote",
"org",
"repo",
"profile_identity",
"session_profile_lock",
"recorded_at",
"updated_at",
"writer_pid",
):
if key in envelope and key not in merged:
merged[key] = envelope[key]
stored = (merged.get("profile_identity") or "").strip()
if stored and stored != profile:
return None
if skip_identity_match:
# Enforce remote/org/repo when caller provides them (#709 F3).
# Drop only *active session* profile-identity mismatches; keep
# expiry, spoof, and repository-scope reasons.
scope_remote = remote if enforce_repo_scope else None
scope_org = org if enforce_repo_scope else None
scope_repo = repo if enforce_repo_scope else None
reasons = identity_match_reasons(
merged,
remote=scope_remote,
org=scope_org,
repo=scope_repo,
profile_identity=stored or profile,
)
filtered = [
r
for r in reasons
if "profile identity mismatch" not in r
or (stored and stored != profile)
]
# When caller requested a specific remote/org/repo, also fail if the
# stored record lacks those identity fields entirely (legacy incomplete).
if enforce_repo_scope:
for field, want in (
("remote", remote),
("org", org),
("repo", repo),
):
want_s = (want or "").strip()
if not want_s:
continue
have = (str(merged.get(field) or "")).strip()
if not have:
filtered.append(
f"session state {field} missing on durable record "
f"(expected={want_s!r}; fail closed, #709 F3)"
)
elif have != want_s:
# identity_match_reasons already adds mismatch; ensure kept
pass
if filtered:
return None
return merged
reasons = identity_match_reasons(
merged,
remote=remote,
org=org,
repo=repo,
profile_identity=profile,
)
if reasons:
return None
return merged
-451
View File
@@ -1,451 +0,0 @@
"""MCP tool-boundary error mapping for known Gitea client failures (#699).
Known authentication / authorization / network / configuration failures leave
the tool boundary as a sanitized structured ``CallToolResult`` with
``isError=True``. Stdio transport remains connected.
Design constraints (reviewer-ratified, #699 / PR #701):
1. **No secret material** in tool results or daemon logs. Messages are fixed
constants keyed by ``reason_code``; HTTP bodies / exception text never
surface. Sanitization failure fails closed to ``internal_error``.
2. **Narrow boundary** wraps the original FastMCP ``Tool.run`` success path;
re-raises framework control-flow exceptions (``UrlElicitationRequiredError``);
maps only typed client failures. Installation is idempotent.
3. **No RuntimeError substring heuristics.** Only explicit typed
authentication / authorization / network / configuration exceptions
receive those labels.
4. **HTTP classification** lives in ``gitea_auth.classify_http_status``;
every 403 is authorization-class.
"""
from __future__ import annotations
import json
import logging
import sys
from typing import Any
logger = logging.getLogger("gitea_mcp.tool_error_boundary")
# Stable reason codes (#699).
REASON_AUTH_FAILED = "auth_failed"
REASON_AUTH_INVALID_TOKEN = "auth_invalid_token"
REASON_AUTHZ_INSUFFICIENT_SCOPE = "authz_insufficient_scope"
REASON_AUTHZ_DENIED = "authz_denied"
REASON_NETWORK_ERROR = "network_error"
REASON_CONFIG_ERROR = "config_error"
REASON_INTERNAL_ERROR = "internal_error"
REASON_UPSTREAM_UNAVAILABLE = "upstream_unavailable"
REASON_HTTP_ERROR = "http_error"
ERROR_CLASS_AUTHENTICATION = "authentication"
ERROR_CLASS_AUTHORIZATION = "authorization"
ERROR_CLASS_NETWORK = "network"
ERROR_CLASS_CONFIGURATION = "configuration"
ERROR_CLASS_INTERNAL = "internal"
ERROR_CLASS_UPSTREAM = "upstream"
# Fixed, secret-free operator messages. Never interpolate HTTP bodies,
# Keychain contents, tokens, or arbitrary exception text.
FIXED_MESSAGES: dict[str, str] = {
REASON_AUTH_FAILED: "Gitea authentication failed",
REASON_AUTH_INVALID_TOKEN: (
"Gitea authentication failed: invalid or revoked credentials"
),
REASON_AUTHZ_INSUFFICIENT_SCOPE: (
"Gitea authorization failed: insufficient token scope"
),
REASON_AUTHZ_DENIED: "Gitea authorization failed: access denied",
REASON_NETWORK_ERROR: "Network error contacting Gitea",
REASON_CONFIG_ERROR: "Gitea configuration or credential resolution failed",
REASON_INTERNAL_ERROR: "Internal tool error",
REASON_UPSTREAM_UNAVAILABLE: "Gitea upstream unavailable",
REASON_HTTP_ERROR: "Gitea HTTP request failed",
}
_INSTALL_FLAG = "_gitea_auth_boundary_installed"
_ORIGINAL_ATTR = "_gitea_auth_boundary_original"
def fixed_message(reason_code: str) -> str:
"""Return the fixed sanitized message for *reason_code* (fail closed)."""
return FIXED_MESSAGES.get(reason_code, FIXED_MESSAGES[REASON_INTERNAL_ERROR])
def _safe_profile_name() -> str | None:
try:
from gitea_auth import get_profile
name = (get_profile() or {}).get("profile_name")
if name is None:
return None
text = str(name).strip()
# Profile names are non-secret identifiers; still bound length.
return text[:80] if text else None
except Exception:
return None
def _is_framework_control_flow(exc: BaseException) -> bool:
"""True for exceptions the MCP framework must re-raise unchanged."""
try:
from mcp.shared.exceptions import UrlElicitationRequiredError
if isinstance(exc, UrlElicitationRequiredError):
return True
except Exception:
pass
# BaseException subclasses that must never become tool isError payloads.
if isinstance(exc, (KeyboardInterrupt, SystemExit, GeneratorExit)):
return True
return False
def is_known_client_failure(exc: BaseException) -> bool:
"""True only for explicit typed Gitea client / config failures.
Never true for arbitrary ``RuntimeError`` (reviewer finding #3).
"""
try:
import gitea_auth
if isinstance(
exc,
(
gitea_auth.GiteaAuthError,
gitea_auth.GiteaAuthzError,
gitea_auth.GiteaNetworkError,
gitea_auth.GiteaConfigError,
gitea_auth.GiteaHttpError,
),
):
return True
except Exception:
return False
try:
import gitea_config
if isinstance(exc, gitea_config.ConfigError):
return True
except Exception:
pass
return False
def classify_exception(exc: BaseException) -> dict[str, Any]:
"""Return structured classification with **fixed** messages only.
Only typed authentication / authorization / network / configuration
failures receive those labels. Unexpected programming failures are
``internal_error``. HTTP bodies and exception text are never copied
into ``message``.
"""
try:
return _classify_exception_impl(exc)
except Exception:
# Fail closed: sanitization / classification failure must not leak.
return {
"reason_code": REASON_INTERNAL_ERROR,
"error_class": ERROR_CLASS_INTERNAL,
"http_status": None,
"message": fixed_message(REASON_INTERNAL_ERROR),
"transport_survives": True,
}
def _classify_exception_impl(exc: BaseException) -> dict[str, Any]:
import gitea_auth
if isinstance(exc, gitea_auth.GiteaAuthError):
code = getattr(exc, "reason_code", None) or REASON_AUTH_INVALID_TOKEN
if code not in (
REASON_AUTH_FAILED,
REASON_AUTH_INVALID_TOKEN,
):
code = REASON_AUTH_INVALID_TOKEN
return {
"reason_code": code,
"error_class": ERROR_CLASS_AUTHENTICATION,
"http_status": getattr(exc, "http_status", None) or 401,
"message": fixed_message(code),
"transport_survives": True,
}
if isinstance(exc, gitea_auth.GiteaAuthzError):
code = getattr(exc, "reason_code", None) or REASON_AUTHZ_DENIED
if code not in (REASON_AUTHZ_DENIED, REASON_AUTHZ_INSUFFICIENT_SCOPE):
code = REASON_AUTHZ_DENIED
return {
"reason_code": code,
"error_class": ERROR_CLASS_AUTHORIZATION,
"http_status": getattr(exc, "http_status", None) or 403,
"message": fixed_message(code),
"transport_survives": True,
}
if isinstance(exc, gitea_auth.GiteaNetworkError):
return {
"reason_code": REASON_NETWORK_ERROR,
"error_class": ERROR_CLASS_NETWORK,
"http_status": getattr(exc, "http_status", None),
"message": fixed_message(REASON_NETWORK_ERROR),
"transport_survives": True,
}
if isinstance(exc, gitea_auth.GiteaConfigError):
return {
"reason_code": REASON_CONFIG_ERROR,
"error_class": ERROR_CLASS_CONFIGURATION,
"http_status": getattr(exc, "http_status", None),
"message": fixed_message(REASON_CONFIG_ERROR),
"transport_survives": True,
}
if isinstance(exc, gitea_auth.GiteaHttpError):
code = getattr(exc, "reason_code", None) or REASON_HTTP_ERROR
if code == REASON_UPSTREAM_UNAVAILABLE:
error_class = ERROR_CLASS_UPSTREAM
else:
error_class = ERROR_CLASS_INTERNAL
code = REASON_HTTP_ERROR
return {
"reason_code": code,
"error_class": error_class,
"http_status": getattr(exc, "http_status", None),
"message": fixed_message(code),
"transport_survives": True,
}
try:
import gitea_config
if isinstance(exc, gitea_config.ConfigError):
return {
"reason_code": REASON_CONFIG_ERROR,
"error_class": ERROR_CLASS_CONFIGURATION,
"http_status": None,
"message": fixed_message(REASON_CONFIG_ERROR),
"transport_survives": True,
}
except Exception:
pass
# Unwrap FastMCP ToolError cause when the original was a typed failure.
cause = getattr(exc, "__cause__", None)
if cause is not None and cause is not exc and is_known_client_failure(cause):
return _classify_exception_impl(cause)
# No message-substring authentication heuristics (reviewer finding #3).
return {
"reason_code": REASON_INTERNAL_ERROR,
"error_class": ERROR_CLASS_INTERNAL,
"http_status": None,
"message": fixed_message(REASON_INTERNAL_ERROR),
"transport_survives": True,
}
def build_structured_error_payload(
classification: dict[str, Any],
*,
tool_name: str | None = None,
profile_name: str | None = None,
) -> dict[str, Any]:
"""LLM-safe structured payload — fixed message + typed metadata only."""
try:
reason = str(classification.get("reason_code") or REASON_INTERNAL_ERROR)
message = fixed_message(reason)
# Refuse to emit any classification message that is not the fixed constant.
if classification.get("message") != message:
message = fixed_message(reason)
payload: dict[str, Any] = {
"success": False,
"isError": True,
"reason_code": reason if reason in FIXED_MESSAGES else REASON_INTERNAL_ERROR,
"error_class": classification.get("error_class") or ERROR_CLASS_INTERNAL,
"message": message,
"transport_survives": True,
"retryable": classification.get("error_class")
in {
ERROR_CLASS_AUTHENTICATION,
ERROR_CLASS_NETWORK,
ERROR_CLASS_CONFIGURATION,
},
}
status = classification.get("http_status")
if isinstance(status, int):
payload["http_status"] = status
if tool_name and isinstance(tool_name, str):
# Tool names are identifiers, not secrets; bound length.
payload["tool"] = tool_name[:120]
if profile_name and isinstance(profile_name, str):
payload["profile"] = profile_name[:80]
return payload
except Exception:
return {
"success": False,
"isError": True,
"reason_code": REASON_INTERNAL_ERROR,
"error_class": ERROR_CLASS_INTERNAL,
"message": fixed_message(REASON_INTERNAL_ERROR),
"transport_survives": True,
"retryable": False,
}
def log_sanitized_daemon_reason(
classification: dict[str, Any],
*,
tool_name: str | None = None,
stream=None,
) -> None:
"""Daemon log: reason codes only — never exception text or response bodies."""
stream = stream if stream is not None else sys.stderr
try:
reason = classification.get("reason_code") or REASON_INTERNAL_ERROR
error_class = classification.get("error_class") or ERROR_CLASS_INTERNAL
# Only emit known tokens; never classification['message'] from callers
# that might have been poisoned.
if reason not in FIXED_MESSAGES:
reason = REASON_INTERNAL_ERROR
error_class = ERROR_CLASS_INTERNAL
parts = [
"mcp_tool_error",
f"reason_code={reason}",
f"error_class={error_class}",
]
if tool_name and isinstance(tool_name, str):
parts.append(f"tool={tool_name[:120]}")
status = classification.get("http_status")
if isinstance(status, int):
parts.append(f"http_status={status}")
# Intentionally no detail= / message= field — secrets lived there.
line = " ".join(parts)
stream.write(line + "\n")
if hasattr(stream, "flush"):
stream.flush()
logger.warning(line)
except Exception:
# Fail closed: never fall back to logging the exception.
try:
stream.write(
"mcp_tool_error reason_code=internal_error "
"error_class=internal\n"
)
if hasattr(stream, "flush"):
stream.flush()
except Exception:
pass
def to_call_tool_result(
exc: BaseException,
*,
tool_name: str | None = None,
profile_name: str | None = None,
log: bool = True,
) -> Any:
"""Build a FastMCP ``CallToolResult`` with ``isError=True`` for *exc*."""
from mcp.types import CallToolResult, TextContent
try:
classification = classify_exception(exc)
if log:
log_sanitized_daemon_reason(classification, tool_name=tool_name)
payload = build_structured_error_payload(
classification, tool_name=tool_name, profile_name=profile_name
)
text = json.dumps(payload, indent=2, sort_keys=True)
return CallToolResult(
content=[TextContent(type="text", text=text)],
structuredContent=payload,
isError=True,
)
except Exception:
# Absolute fail-closed path — no exception text.
fallback = {
"success": False,
"isError": True,
"reason_code": REASON_INTERNAL_ERROR,
"error_class": ERROR_CLASS_INTERNAL,
"message": fixed_message(REASON_INTERNAL_ERROR),
"transport_survives": True,
"retryable": False,
}
return CallToolResult(
content=[
TextContent(
type="text",
text=json.dumps(fallback, indent=2, sort_keys=True),
)
],
structuredContent=fallback,
isError=True,
)
def install_tool_run_boundary(Tool) -> bool:
"""Install a **narrow** error boundary around FastMCP ``Tool.run``.
Strategy (preserves framework semantics):
* Call the **original** ``Tool.run`` for the success path (async, return
types, convert_result, protocol behavior unchanged).
* Re-raise ``UrlElicitationRequiredError`` and other control-flow
exceptions without mapping to ``internal_error``.
* Map typed Gitea client failures (and ToolError whose ``__cause__`` is
typed) to structured ``CallToolResult(isError=True)``.
* Map remaining unexpected tool failures to fixed ``internal_error``
isError results (transport survival) without secret-bearing text.
* Idempotent: second install is a no-op and returns ``False``.
"""
if getattr(Tool.run, _INSTALL_FLAG, False):
return False
from mcp.server.fastmcp.exceptions import ToolError
from mcp.shared.exceptions import UrlElicitationRequiredError
original_run = Tool.run
async def run_boundary(
self,
arguments: dict[str, Any],
context=None,
convert_result: bool = False,
) -> Any:
try:
return await original_run(
self,
arguments,
context=context,
convert_result=convert_result,
)
except UrlElicitationRequiredError:
# Framework control-flow — must not become internal_error.
raise
except BaseException as exc:
if _is_framework_control_flow(exc):
raise
if not isinstance(exc, Exception):
raise
# Prefer typed cause under FastMCP ToolError wrappers.
target: BaseException = exc
if isinstance(exc, ToolError) and exc.__cause__ is not None:
target = exc.__cause__
# Known client failures → structured isError with reason codes.
# Unexpected failures → fixed internal_error isError (no secrets).
profile_name = _safe_profile_name()
return to_call_tool_result(
target,
tool_name=getattr(self, "name", None),
profile_name=profile_name,
)
setattr(run_boundary, _INSTALL_FLAG, True)
setattr(run_boundary, _ORIGINAL_ATTR, original_run)
Tool.run = run_boundary # type: ignore[method-assign]
return True
def boundary_is_installed(Tool) -> bool:
"""True when the #699 boundary is active on *Tool.run*."""
return bool(getattr(Tool.run, _INSTALL_FLAG, False))
-4
View File
@@ -18,13 +18,11 @@ DEFAULT_ADOPTION_REASON = "merger-handoff-approved-head"
SOURCE_ADOPT = "gitea_adopt_merger_pr_lease"
SOURCE_ACQUIRE = "gitea_acquire_reviewer_pr_lease"
SOURCE_ACQUIRE_MERGER = "gitea_acquire_merger_pr_lease"
SOURCE_HEARTBEAT = "gitea_heartbeat_reviewer_pr_lease"
SANCTIONED_PROVENANCE_SOURCES = frozenset({
SOURCE_ADOPT,
SOURCE_ACQUIRE,
SOURCE_ACQUIRE_MERGER,
SOURCE_HEARTBEAT,
})
@@ -211,10 +209,8 @@ _SANCTIONED_LEASE_EVIDENCE_RE = re.compile(
r"(?is)\b("
r"gitea_adopt_merger_pr_lease|"
r"gitea_acquire_reviewer_pr_lease|"
r"gitea_acquire_merger_pr_lease|"
r"lease_proof_source\s*[:=]\s*gitea_adopt_merger_pr_lease|"
r"lease_proof_source\s*[:=]\s*gitea_acquire_reviewer_pr_lease|"
r"lease_proof_source\s*[:=]\s*gitea_acquire_merger_pr_lease|"
r"lease_proof_kind\s*[:=]\s*sanctioned_adoption|"
r"lease_proof_kind\s*[:=]\s*sanctioned_acquire|"
r"adoption_comment_id\s*[:=]|"
+7 -126
View File
@@ -20,114 +20,12 @@ if PROJECT_ROOT not in sys.path:
import gitea_config
# Defaults emit *canonical* operation names only. Shorthand that is not in
# gitea_config.GITEA_OPERATION_ALIASES (e.g. ``pr.close``, ``issue.close``)
# is silently dropped by the production loader and must never appear here.
AUTHOR_DEFAULT_ALLOWED = [
"gitea.read",
"gitea.branch.create",
"gitea.repo.commit",
"gitea.branch.push",
"gitea.pr.create",
"gitea.pr.comment",
]
AUTHOR_DEFAULT_FORBIDDEN = [
"gitea.pr.approve",
"gitea.pr.request_changes",
"gitea.pr.merge",
]
AUTHOR_DEFAULT_ALLOWED = ["read", "branch", "commit", "push", "open_pr", "comment"]
AUTHOR_DEFAULT_FORBIDDEN = ["approve", "request_changes", "merge"]
REVIEWER_DEFAULT_ALLOWED = [
"gitea.read",
"gitea.pr.review",
"gitea.pr.comment",
"gitea.pr.approve",
"gitea.pr.request_changes",
"gitea.pr.merge",
"read", "review", "comment", "approve", "request_changes", "merge"
]
REVIEWER_DEFAULT_FORBIDDEN = [
"gitea.branch.create",
"gitea.repo.commit",
"gitea.branch.push",
"gitea.pr.create",
]
# Required reconciler ops (read + pr.close) plus recommended comment/close and
# branch.delete for guarded merged-PR cleanup. All names must normalize via
# gitea_config.normalize_operation without being dropped.
RECONCILER_DEFAULT_ALLOWED = [
"gitea.read",
"gitea.pr.close",
"gitea.pr.comment",
"gitea.issue.comment",
"gitea.issue.close",
"gitea.branch.delete",
]
RECONCILER_DEFAULT_FORBIDDEN = [
"gitea.pr.approve",
"gitea.pr.merge",
"gitea.pr.review",
"gitea.pr.create",
"gitea.branch.push",
"gitea.repo.commit",
]
# Migration-only expansions for common shorthands that are *not* in
# GITEA_OPERATION_ALIASES. Emitted output is always the canonical form so a
# second canonicalize pass is a no-op (idempotent).
_MIGRATION_ONLY_ALIASES = {
"pr.close": "gitea.pr.close",
"pr.comment": "gitea.pr.comment",
"issue.close": "gitea.issue.close",
"branch.delete": "gitea.branch.delete",
}
# Reconciler required ops that must survive migration (from reconciler_profile).
RECONCILER_REQUIRED_CANONICAL = ("gitea.read", "gitea.pr.close")
def canonicalize_operation(op: str) -> str:
"""Return a canonical operation name accepted by the production loader.
Fail closed on unknown/ambiguous spellings so required permissions cannot
be silently dropped by ``check_operation`` later.
"""
if not isinstance(op, str) or not op.strip():
raise ValueError("operation must be a non-empty string (fail closed)")
op = op.strip()
try:
return gitea_config.normalize_operation(op)
except gitea_config.ConfigError:
pass
if op in _MIGRATION_ONLY_ALIASES:
return _MIGRATION_ONLY_ALIASES[op]
raise ValueError(
f"operation {op!r} cannot be canonicalized for migration "
"(unknown/ambiguous; fail closed — production loader would drop it)"
)
def canonicalize_operations(ops, *, context: str = "operations") -> list[str]:
"""Canonicalize a list of operations; preserve order, drop duplicates."""
if not isinstance(ops, list):
raise ValueError(f"{context} must be a list (fail closed)")
out: list[str] = []
seen: set[str] = set()
for entry in ops:
canon = canonicalize_operation(entry)
if canon not in seen:
seen.add(canon)
out.append(canon)
return out
def _assert_reconciler_required_survive(allowed: list[str], profile_name: str) -> None:
"""Fail visibly when migration would leave a reconciler without required ops."""
missing = [op for op in RECONCILER_REQUIRED_CANONICAL if op not in set(allowed)]
if missing:
raise ValueError(
f"Profile '{profile_name}' (reconciler) is missing required "
f"operation(s) after migration: {missing}. Refusing to emit a "
"profile that would silently fail pr.close / read (fail closed)."
)
REVIEWER_DEFAULT_FORBIDDEN = ["branch", "commit", "push", "open_pr"]
def infer_role(name, execution_profile):
@@ -192,11 +90,9 @@ def migrate_v1_to_v2(v1_data):
ident_name = "reviewer"
elif role == "author":
ident_name = "author"
elif role == "reconciler":
ident_name = "reconciler"
else:
role = prof.get("role")
if role not in (None, "author", "reviewer", "reconciler"):
if role not in (None, "author", "reviewer"):
raise ValueError(
f"Profile '{name}' has unsupported role {role!r}"
)
@@ -228,35 +124,20 @@ def migrate_v1_to_v2(v1_data):
raise ValueError(
f"Profile '{name}' operation fields must be lists"
)
try:
identity_data["allowed_operations"] = canonicalize_operations(
allowed, context=f"profile '{name}' allowed_operations"
)
identity_data["forbidden_operations"] = canonicalize_operations(
forbidden, context=f"profile '{name}' forbidden_operations"
)
except ValueError as exc:
raise ValueError(f"Profile '{name}': {exc}") from exc
identity_data["allowed_operations"] = list(allowed)
identity_data["forbidden_operations"] = list(forbidden)
elif role == "author":
identity_data["allowed_operations"] = list(AUTHOR_DEFAULT_ALLOWED)
identity_data["forbidden_operations"] = list(AUTHOR_DEFAULT_FORBIDDEN)
elif role == "reviewer":
identity_data["allowed_operations"] = list(REVIEWER_DEFAULT_ALLOWED)
identity_data["forbidden_operations"] = list(REVIEWER_DEFAULT_FORBIDDEN)
elif role == "reconciler":
identity_data["allowed_operations"] = list(RECONCILER_DEFAULT_ALLOWED)
identity_data["forbidden_operations"] = list(RECONCILER_DEFAULT_FORBIDDEN)
else:
raise ValueError(
f"Profile '{name}' has no explicit operation lists and no "
"unambiguous author/reviewer role marker (fail closed)"
)
if role == "reconciler":
_assert_reconciler_required_survive(
identity_data["allowed_operations"], name
)
# Nest inside environments/services structure
env = environments.setdefault(env_name, {})
services = env.setdefault("services", {})
+8 -27
View File
@@ -86,22 +86,6 @@ _WRONG_BRANCH_RE = re.compile(
r"deleted branch (?:does not match|!=|differs from) (?:merged )?pr head",
re.IGNORECASE,
)
# #698: canonical reviewer lease lifecycle operations are NOT post-merge
# cleanup. Releasing a reviewer PR lease (or posting its terminal
# phase=released marker) happens after every review — merged or not — and
# must never trigger the post-merge branch/worktree cleanup checklist.
_LEASE_LIFECYCLE_RE = re.compile(
r"(?:gitea_release_reviewer_pr_lease|gitea_abandon_workflow_lease|"
r"release[d]? (?:the )?(?:reviewer|workflow) (?:pr )?lease|"
r"reviewer (?:pr )?lease release[d]?|"
r"lease (?:marker|comment).{0,40}phase\s*[:=]\s*released|"
r"phase\s*[:=]\s*released)",
re.IGNORECASE,
)
_CLEANUP_MUTATIONS_VALUE_RE = re.compile(
r"cleanup mutations\s*:\s*([^\n]+)",
re.IGNORECASE,
)
def _claims_remote_delete(text: str) -> bool:
@@ -220,19 +204,16 @@ def assess_post_merge_cleanup_proof(
for field in _worktree_cleanup_fields_present(text)
)
if (remote_delete or worktree_remove) and not (remote_delete or worktree_remove):
pass
if not remote_delete and not worktree_remove:
value_match = _CLEANUP_MUTATIONS_VALUE_RE.search(text)
value = (value_match.group(1).strip() if value_match else "")
value_lower = value.lower()
substantive = bool(value) and value_lower not in {
"none", "n/a", "not applicable",
}
# #698: reviewer lease release / terminal lease markers are lease
# lifecycle, not post-merge cleanup — no checklist owed.
lease_lifecycle_only = substantive and bool(
_LEASE_LIFECYCLE_RE.search(value)
cleanup_mutations = re.search(
r"cleanup mutations\s*:\s*(?!none\b)\S",
text,
re.IGNORECASE,
)
if substantive and not lease_lifecycle_only:
if cleanup_mutations:
reasons.append(
"cleanup mutations reported without post-merge cleanup proof checklist"
)
+6 -67
View File
@@ -403,37 +403,10 @@ _REVIEWER_ACTIVE_RE = re.compile(
r"whether any reviewer was active\s*:\s*(yes|no|true|false)",
re.IGNORECASE,
)
# #698 phase detection for phase-specific head proofs.
_NO_REVIEWED_HEAD_RE = re.compile(
r"(?:reviewed head sha|candidate head sha)\s*:\s*none\b",
re.IGNORECASE,
)
_VERDICT_RECORDED_RE = re.compile(
r"review decision\s*:\s*(?:approve[d]?|request[_ ]changes)\b"
r"|review_status\s*:\s*(?:approved|request_changes)\b"
r"|terminal review mutation\s*:\s*(?!none\b)\S",
re.IGNORECASE,
)
_MERGE_ATTEMPTED_RE = re.compile(
r"merge result\s*:\s*(?:merged|success|performed|failed|attempted)\b"
r"|merge mutations\s*:\s*(?!none\b|not applicable\b)\S",
re.IGNORECASE,
)
_VALIDATION_STARTED_RE = re.compile(
r"validation\s*:\s*(?!none\b|not run\b|not applicable\b|not started\b)"
r"[^\n]*(?:pass|fail|ran|executed|\d+\s+passed)",
re.IGNORECASE,
)
def assess_reviewer_stale_head_final_report(report_text: str) -> dict[str, Any]:
"""Final-report proof for reviewed vs live head SHAs (#399 AC 6).
#698: head proofs are phase-specific. A legitimately blocked run that
never began validation (no reviewed head, no formal verdict, no merge)
owes none of them; approval-time and merge-time live-head proofs are
owed only once the corresponding phase actually begins.
"""
"""Final-report proof for reviewed vs live head SHAs (#399 AC 6)."""
text = report_text or ""
reasons: list[str] = []
reviewed = _normalize_sha(_REVIEWED_HEAD_RE.search(text).group(1) if _REVIEWED_HEAD_RE.search(text) else None)
@@ -449,49 +422,15 @@ def assess_reviewer_stale_head_final_report(report_text: str) -> dict[str, Any]:
)
push_during = _PUSH_DURING_VALIDATION_RE.search(text)
# Phase detection from the report's own claims.
no_head_stated = bool(_NO_REVIEWED_HEAD_RE.search(text))
verdict_recorded = bool(_VERDICT_RECORDED_RE.search(text))
merge_attempted = bool(_MERGE_ATTEMPTED_RE.search(text))
validation_started = bool(reviewed) or bool(_VALIDATION_STARTED_RE.search(text))
blocked_before_validation = (
no_head_stated
and not reviewed
and not verdict_recorded
and not merge_attempted
and not validation_started
)
if blocked_before_validation:
return {
"proven": True,
"block": False,
"reasons": [],
"reviewed_head_sha": None,
"live_head_sha_before_approval": None,
"live_head_sha_before_merge": None,
"push_during_validation": (
push_during.group(1).lower() if push_during else None
),
"phase": "blocked_before_validation",
}
if not reviewed and not no_head_stated:
# The head must always be STATED — either a SHA or an explicit
# 'none'. Silence is not a phase claim and fails closed.
reasons.append(
"reviewed head SHA not stated in final report "
"(state the SHA or an explicit 'none')"
)
elif not reviewed and (validation_started or verdict_recorded or merge_attempted):
if not reviewed:
reasons.append("reviewed head SHA not stated in final report")
if verdict_recorded and not live_approval:
if not live_approval:
reasons.append("final live head SHA before approval not stated")
if merge_attempted and not live_merge:
if not live_merge:
reasons.append("final live head SHA before merge not stated")
if validation_started and not push_during:
if not push_during:
reasons.append("whether push occurred during validation not stated")
if reviewed and live_approval and reviewed != live_approval:
elif reviewed and live_approval and reviewed != live_approval:
reasons.append("live head before approval differs from reviewed head SHA")
elif reviewed and live_merge and reviewed != live_merge:
reasons.append("live head before merge differs from reviewed head SHA")
-5
View File
@@ -18,11 +18,6 @@ RECONCILER_RECOMMENDED_OPERATIONS = (
"gitea.pr.comment",
"gitea.issue.comment",
"gitea.issue.close",
# Merged-branch cleanup is reconciler-owned (task_capability_map maps
# cleanup_merged_pr_branch -> reconciler). The permission is only
# exercisable through the guarded gitea_cleanup_merged_pr_branch path
# (#514): merged proof, protected-branch refusal, explicit confirmation.
"gitea.branch.delete",
)
RECONCILER_FORBIDDEN_OPERATIONS = (
+1 -6
View File
@@ -25,13 +25,8 @@ _REVIEWED_HEAD_RE = re.compile(
r"(?:pinned reviewed head|reviewed head sha)\s*:\s*([0-9a-f]{7,40})",
re.IGNORECASE,
)
# #698: validation pass proof appears in several legitimate shapes —
# "Validation: pass", "Validation: focused 50 passed; full 2665 passed",
# or structured "validation_status: pass". Accept pass evidence anywhere in
# the Validation field's value, not only as its first token.
_VALIDATION_PASS_RE = re.compile(
r"validation(?:_status)?\s*:[^\n]{0,300}?"
r"(?:\bpass(?:ed)?\b|\bstrong\b|\bok\b|\bgreen\b|\d+\s+passed)",
r"validation\s*:\s*(?:pass|passed|strong|ok|green)",
re.IGNORECASE,
)
_MERGED_CLAIM_RE = re.compile(
+9 -36
View File
@@ -858,16 +858,9 @@ _WALKTHROUGH_ARTIFACT_RE = re.compile(r"walkthrough\.md", re.I)
def _performed_file_mutations(action_log: list[dict] | None) -> list[dict]:
"""Return performed local file mutations, excluding gated rejections.
Non-dict entries (malformed JSON, LLM mistakes) are ignored instead of
raising ``AttributeError`` (#698): a malformed ledger entry can never be
authoritative mutation evidence.
"""
"""Return performed local file mutations, excluding gated rejections."""
performed: list[dict] = []
for entry in action_log or []:
if not isinstance(entry, dict):
continue
if entry.get("gated_rejected") or entry.get("performed") is False:
continue
action = (entry.get("action") or "").strip().lower()
@@ -2235,31 +2228,24 @@ HANDOFF_REVIEW_MUTATION_FIELDS = (
)
HANDOFF_ROLE_FIELDS = {
# #698: the review/merger required-field sets must stay aligned with the
# canonical schema (skills/llm-project-workflow/schemas/
# review-merge-final-report.md). The schema explicitly FORBIDS the legacy
# fields 'Pinned reviewed head', 'Scratch worktree used', and 'Workspace
# mutations' — a validator must never demand a field the schema bans.
"review": (
("Selected PR", ("selected pr",)),
("Reviewer eligibility", ("reviewer eligibility", "eligibility")),
("Reviewed head SHA", ("reviewed head sha", "candidate head sha")),
("Review worktree path", ("review worktree path", "worktree path",
"starting worktree path")),
("Review worktree dirty", ("review worktree dirty", "worktree dirty",
"whether worktree was dirty")),
("Pinned reviewed head", ("pinned reviewed head", "pinned head")),
("Worktree path", ("worktree path", "starting worktree path")),
("Worktree dirty", ("worktree dirty", "whether worktree was dirty")),
("Scratch worktree used", ("scratch worktree used", "scratch clone used",
"scratch worktree")),
("Unrelated local mutations", ("unrelated local mutations",
"unrelated files modified",
"file edits by reviewer")),
"unrelated files modified")),
("Review decision", ("review decision", "decision")),
("Merge result", ("merge result",)),
("Linked issue status", ("linked issue status", "linked issue")),
("Cleanup status", ("cleanup status", "cleanup")),
("Safe next action", ("safe next action", "next")),
) + HANDOFF_REVIEW_MUTATION_FIELDS,
"merger": (
("Selected PR", ("selected pr",)),
("Reviewed head SHA", ("reviewed head sha", "candidate head sha")),
("Pinned reviewed head", ("pinned reviewed head", "pinned head")),
("Active profile", ("active profile",)),
("Role kind", ("role kind",)),
("Merge capability source", ("merge capability source",)),
@@ -2447,22 +2433,9 @@ def assess_controller_handoff(report_text, role=None, local_edits=False):
# Issue #320: reviewer and merger handoffs use the precise mutation categories
# in HANDOFF_REVIEW_MUTATION_FIELDS instead of the legacy ambiguous
# "Workspace mutations" field, which is rejected below.
# Issue #698: the canonical review-merge schema has no 'Mutations',
# 'Next', 'Issue/PR', 'Branch/SHA', or 'Files changed' fields — their
# content lives in the precise mutation categories, 'Safe next
# action', 'Selected PR'/'Linked issue', head-SHA fields, and 'Files
# reviewed'. Requiring the legacy names rejects canonical reports.
_non_canonical_for_review = {
"Workspace mutations",
"Mutations",
"Next",
"Issue/PR",
"Branch/SHA",
"Files changed",
}
required = [
field for field in required
if field[0] not in _non_canonical_for_review
if field[0] != "Workspace mutations"
]
if any(label.startswith("workspace mutations") for label in labels):
return {
-249
View File
@@ -438,252 +438,3 @@ def format_cleanup_audit_comment(audit: dict[str, Any]) -> str:
"This path only clears a lock when the referenced PR is merged/closed.",
]
return "\n".join(lines)
# ---------------------------------------------------------------------------
# #709 cross-profile cleanup, overwrite protection, recovery provenance
# ---------------------------------------------------------------------------
def has_unresolved_terminal_evidence(lock: dict | None) -> bool:
"""True when *lock* still carries a terminal live mutation ledger."""
return last_terminal_mutation(lock) is not None
def assess_init_overwrite(
existing_lock: dict | None,
*,
force: bool = False,
) -> dict[str, Any]:
"""Whether init_review_decision_lock may replace an existing durable lock (#709 AC2).
Unresolved terminal evidence must never be silently replaced by an empty
initialized lock. *force* does not authorize destruction of terminal
ledgers only explicit cleanup / archive transitions may remove them.
"""
result: dict[str, Any] = {
"overwrite_allowed": True,
"has_existing": existing_lock is not None,
"has_unresolved_terminal": False,
"reasons": [],
"last_terminal_pr": None,
"last_terminal_action": None,
"existing_profile_identity": None,
}
if existing_lock is None:
result["reasons"].append("no existing lock; empty init allowed")
return result
result["existing_profile_identity"] = (
existing_lock.get("profile_identity")
or existing_lock.get("session_profile_lock")
or existing_lock.get("session_profile")
)
last = last_terminal_mutation(existing_lock)
if last is None:
result["reasons"].append(
"existing lock has no terminal mutations; re-init allowed"
)
return result
result["has_unresolved_terminal"] = True
result["overwrite_allowed"] = False
result["last_terminal_pr"] = last.get("pr_number")
result["last_terminal_action"] = last.get("action")
result["reasons"].append(
"refuse empty re-init: unresolved terminal decision-lock evidence "
f"present ({last.get('action')} on PR #{last.get('pr_number')}); "
"use gitea_cleanup_stale_review_decision_lock when moot, or archive "
f"via sanctioned recovery — force={force!r} does not authorize overwrite "
"(#709 AC2)"
)
return result
def lock_targets_merged_pr_approval(
lock: dict | None,
*,
pr_number: int,
expected_head_sha: str | None = None,
) -> bool:
"""True when *lock*'s last terminal mutation is approve of *pr_number*.
When *expected_head_sha* is provided, a **recorded** terminal head must
match. Legacy same-repo approve locks with no recorded head do **not**
match (fail closed, #709 F3 residual / review 435) — callers must use the
strict secondary path that refuses no-head clears.
"""
last = last_terminal_mutation(lock)
if last is None:
return False
if last.get("action") != "approve":
return False
if last.get("pr_number") != pr_number:
return False
if expected_head_sha:
want = normalize_head_sha(expected_head_sha)
if not want:
return False
locked = mutation_head_sha(last, lock)
# Require recorded-head match; unrecorded head is not a match (#709 F3).
if not locked or not heads_equal(locked, want):
return False
return True
def build_post_merge_recovery_record(
*,
pr_number: int,
head_sha: str | None,
merge_commit_sha: str | None,
target_profile_identity: str | None,
failed_step: str,
error: str | None,
remote: str | None,
org: str | None,
repo: str | None,
actor_username: str | None,
profile_name: str | None,
) -> dict[str, Any]:
"""Durable recovery-required payload after irreversible merge (#709 AC3)."""
return {
"event": "post_merge_decision_lock_recovery_required",
"status": "recovery_required",
"issue_ref": "#709",
"recovery_critical": True,
"applied": False, # never claim historical cleanup succeeded
"timestamp": datetime.now(timezone.utc).isoformat(),
"pr_number": pr_number,
"head_sha": normalize_head_sha(head_sha),
"merge_commit_sha": merge_commit_sha,
"target_profile_identity": target_profile_identity,
"failed_step": failed_step,
"error": error,
"remote": remote,
"org": org,
"repo": repo,
"actor_username": actor_username,
"profile_name": profile_name,
"required_recovery_action": (
"retry cross-profile decision-lock cleanup and audit publication "
"for the merged PR; if terminal evidence is gone, use "
"gitea_record_irrecoverable_decision_lock_provenance"
),
}
def build_irrecoverable_provenance_record(
*,
pr_number: int,
head_sha: str | None,
remote: str | None,
org: str | None,
repo: str | None,
actor_username: str | None,
profile_name: str | None,
reason: str,
incident_ref: str | None = None,
# Deprecated kwargs retained only so stale call sites fail closed:
operator_authorized: bool | None = None,
# Required for merger-acceptable records (#709 F1):
authorization: dict[str, Any] | None = None,
incident_issue: int | None = None,
incident_comment_id: int | None = None,
destroyed_subject: str | None = None,
historical_provenance_subject: str | None = None,
) -> dict[str, Any]:
"""Truthful absence-of-proof record (#709 AC5). Never sets applied=True.
Caller-supplied ``operator_authorized`` is **ignored** as authorization
evidence (review 434 F1). Prefer
:func:`irrecoverable_provenance.build_irrecoverable_provenance_record`
with a verified server-side authorization artifact.
"""
# Explicitly ignore deprecated self-assertable Boolean.
_ = operator_authorized
if authorization is not None and incident_issue is not None and incident_comment_id is not None:
from irrecoverable_provenance import (
build_irrecoverable_provenance_record as _build,
)
return _build(
pr_number=int(pr_number),
head_sha=str(head_sha or ""),
remote=str(remote or ""),
org=str(org or ""),
repo=str(repo or ""),
actor_username=actor_username,
profile_name=profile_name,
reason=reason,
incident_issue=int(incident_issue),
incident_comment_id=int(incident_comment_id),
authorization=authorization,
destroyed_subject=destroyed_subject,
historical_provenance_subject=historical_provenance_subject
or destroyed_subject,
)
# Fail-closed skeleton when no server authorization is supplied: never
# sets merger_may_accept True (even if operator_authorized was True).
return {
"event": "irrecoverable_decision_lock_provenance",
"status": "provenance_irrecoverable",
"record_type": "irrecoverable_decision_provenance",
"operator_recovery_required": True,
"issue_ref": "#709",
"recovery_critical": True,
"applied": False,
"historical_cleanup_proven": False,
"timestamp": datetime.now(timezone.utc).isoformat(),
"pr_number": pr_number,
"head_sha": normalize_head_sha(head_sha),
"remote": remote,
"org": org,
"repo": repo,
"actor_username": actor_username,
"profile_name": profile_name,
"reason": reason,
"incident_ref": incident_ref,
"incident_issue": incident_issue,
"incident_comment_id": incident_comment_id,
"authorization_verified": False,
"merger_may_accept": False,
"acceptance_rule": (
"Merger may accept this record only when a server-side "
"authorization artifact verifies for remote/org/repo/PR/exact "
"head/incident, the record is durable and read back, and normal "
"merge gates still pass. Caller Booleans never authorize. This "
"does not prove historical cleanup."
),
}
def format_irrecoverable_audit_comment(record: dict[str, Any]) -> str:
"""Markdown body for irrecoverable provenance audit (no applied=true claim)."""
from irrecoverable_provenance import (
format_irrecoverable_audit_comment as _fmt,
)
return _fmt(record)
def format_post_merge_recovery_comment(record: dict[str, Any]) -> str:
"""Markdown body for post-merge recovery-required audit."""
lines = [
"## Post-merge decision-lock recovery required (#709)",
"",
"Status: **RECOVERY_REQUIRED** (merge is irreversible; cleanup/audit incomplete)",
"",
f"- actor: `{record.get('actor_username')}`",
f"- profile: `{record.get('profile_name')}`",
f"- timestamp: `{record.get('timestamp')}`",
f"- PR: `#{record.get('pr_number')}`",
f"- head_sha: `{record.get('head_sha')}`",
f"- merge_commit_sha: `{record.get('merge_commit_sha')}`",
f"- target_profile_identity: `{record.get('target_profile_identity')}`",
f"- failed_step: `{record.get('failed_step')}`",
f"- error: `{record.get('error')}`",
"",
f"Required action: {record.get('required_recovery_action')}",
"",
"applied=false — do not treat this as successful cleanup evidence.",
]
return "\n".join(lines)
+1 -31
View File
@@ -84,11 +84,7 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = {
},
"adopt_merger_pr_lease": {
"permission": "gitea.pr.comment",
"role": "merger",
},
"acquire_merger_pr_lease": {
"permission": "gitea.pr.comment",
"role": "merger",
"role": "reviewer",
},
# #691: guarded non-owner cleanup of obsolete comment-backed reviewer leases.
# Apply path posts lease release + audit comments (gitea.pr.comment).
@@ -131,32 +127,6 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = {
"permission": "gitea.pr.review",
"role": "reviewer",
},
# #709: truthful absence-of-proof recovery (server-side auth + record + consume).
# Dedicated mutation capability — gitea.read is insufficient (review 434 F1).
"issue_irrecoverable_provenance_authorization": {
"permission": "gitea.decision_lock.irrecoverable_recovery",
"role": "reconciler",
},
"gitea_issue_irrecoverable_provenance_authorization": {
"permission": "gitea.decision_lock.irrecoverable_recovery",
"role": "reconciler",
},
"record_irrecoverable_decision_lock_provenance": {
"permission": "gitea.decision_lock.irrecoverable_recovery",
"role": "reconciler",
},
"gitea_record_irrecoverable_decision_lock_provenance": {
"permission": "gitea.decision_lock.irrecoverable_recovery",
"role": "reconciler",
},
"consume_irrecoverable_decision_lock_provenance": {
"permission": "gitea.pr.merge",
"role": "merger",
},
"gitea_consume_irrecoverable_decision_lock_provenance": {
"permission": "gitea.pr.merge",
"role": "merger",
},
"delete_branch": {
"permission": "gitea.branch.delete",
"role": "author",
-11
View File
@@ -58,17 +58,6 @@ def _reset_mutation_authority(monkeypatch):
_fallback: str = state_dir,
_env_key: str = mcp_session_state.STATE_DIR_ENV,
) -> str:
# #695 AC2: when production native transport has pinned a session
# state root, that pin is authoritative even under test isolation
# (PR #701 redirected-state regression).
try:
import mcp_daemon_guard
pinned = mcp_daemon_guard.pinned_session_state_dir()
if pinned:
return pinned
except Exception:
pass
raw = (os.environ.get(_env_key) or "").strip()
return raw or _fallback
+16 -22
View File
@@ -79,46 +79,41 @@ class TestApiRequestFailures(unittest.TestCase):
@patch("gitea_auth.urllib.request.urlopen")
def test_timeout_converted_to_runtimeerror(self, mock_open):
mock_open.side_effect = TimeoutError("timed out")
with self.assertRaises(gitea_auth.GiteaNetworkError) as ctx:
with self.assertRaises(RuntimeError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
# Fixed message only (#699) — still a RuntimeError subclass.
self.assertIsInstance(ctx.exception, RuntimeError)
self.assertEqual(str(ctx.exception), "Network error contacting Gitea")
self.assertIn("network error contacting Gitea", str(ctx.exception))
@patch("gitea_auth.urllib.request.urlopen")
def test_dns_network_failure_converted(self, mock_open):
mock_open.side_effect = urllib.error.URLError("Name or service not known")
with self.assertRaises(gitea_auth.GiteaNetworkError) as ctx:
with self.assertRaises(RuntimeError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertEqual(str(ctx.exception), "Network error contacting Gitea")
self.assertIn("network error contacting Gitea", str(ctx.exception))
@patch("gitea_auth.urllib.request.urlopen")
def test_502_upstream_message(self, mock_open):
mock_open.side_effect = http_error(502, "bad gateway")
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
with self.assertRaises(RuntimeError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
msg = str(ctx.exception)
self.assertEqual(msg, "Gitea upstream unavailable")
self.assertEqual(ctx.exception.reason_code, "upstream_unavailable")
self.assertEqual(ctx.exception.http_status, 502)
self.assertIn("HTTP 502", msg)
self.assertIn("upstream unavailable", msg)
@patch("gitea_auth.urllib.request.urlopen")
def test_503_upstream_message(self, mock_open):
mock_open.side_effect = http_error(503, "")
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
with self.assertRaises(RuntimeError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertEqual(str(ctx.exception), "Gitea upstream unavailable")
self.assertEqual(ctx.exception.http_status, 503)
self.assertIn("HTTP 503", str(ctx.exception))
self.assertIn("upstream unavailable", str(ctx.exception))
@patch("gitea_auth.urllib.request.urlopen")
def test_malformed_error_payload_does_not_crash(self, mock_open):
# Non-JSON garbage error body must still yield a clean typed error
# with a fixed message (no body echo — #699).
# Non-JSON garbage error body must still yield a clean RuntimeError.
mock_open.side_effect = http_error(500, "<html>garbage</html>")
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
with self.assertRaises(RuntimeError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertEqual(str(ctx.exception), "Gitea HTTP request failed")
self.assertNotIn("<html>", str(ctx.exception))
self.assertIn("HTTP 500", str(ctx.exception))
@patch("gitea_auth.urllib.request.urlopen")
def test_malformed_success_json_raises_clean_error(self, mock_open):
@@ -131,17 +126,16 @@ class TestApiRequestFailures(unittest.TestCase):
def test_no_secret_leak_in_error_body(self, mock_open):
mock_open.side_effect = http_error(
400, "failed: token supersecret123 rejected")
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
with self.assertRaises(RuntimeError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
msg = str(ctx.exception)
self.assertNotIn("supersecret123", msg)
# Fixed message — body never appears (stronger than redaction).
self.assertEqual(msg, "Gitea HTTP request failed")
self.assertIn(gitea_audit.REDACTED, msg)
@patch("gitea_auth.urllib.request.urlopen")
def test_auth_header_never_in_error(self, mock_open):
mock_open.side_effect = http_error(400, "bad request")
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
with self.assertRaises(RuntimeError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertNotIn(FAKE_AUTH, str(ctx.exception))
+1 -7
View File
@@ -27,13 +27,7 @@ from task_capability_map import required_permission, required_role
DELETE_PROFILE = {
"profile_name": "prgs-author-delete",
"role": "author",
"allowed_operations": [
"gitea.read",
"gitea.pr.create",
"gitea.branch.push",
"gitea.branch.delete",
],
"allowed_operations": ["gitea.read", "gitea.branch.delete"],
"forbidden_operations": [],
"audit_label": "prgs-author-delete",
}
File diff suppressed because it is too large Load Diff
@@ -1,18 +1,14 @@
"""Regression tests for Issue #695 — second incident (PR #694 / review 427).
Reproduces offline import, env-only runtime spoof, exposed-token invocation,
direct imports, basename entrypoint spoof, allow_test_bootstrap forgery,
standalone quarantine attempts, and false official workflow canonical claims.
Gates must fail closed.
direct imports, locally generated runtime keys, standalone quarantine attempts,
and false official workflow canonical claims. Gates must fail closed.
"""
from __future__ import annotations
import os
import subprocess
import sys
import tempfile
import textwrap
import unittest
from pathlib import Path
from unittest.mock import patch
@@ -24,26 +20,6 @@ import canonical_comment_validator as ccv
HEAD_694 = "1844e298809373be19a526fd39b7d8b0669eb5bd"
HEAD_OTHER = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
REPO_ROOT = Path(__file__).resolve().parent.parent
def _run_offline_snippet(snippet: str, *, env_extra: dict[str, str] | None = None) -> subprocess.CompletedProcess:
"""Execute snippet in a fresh interpreter (no pytest modules)."""
env = os.environ.copy()
env.pop("PYTEST_CURRENT_TEST", None)
env.pop(mcp_daemon_guard.FORCE_PROVENANCE_FAIL_ENV, None)
env["PYTHONPATH"] = str(REPO_ROOT) + os.pathsep + env.get("PYTHONPATH", "")
if env_extra:
env.update(env_extra)
return subprocess.run(
[sys.executable, "-c", snippet],
capture_output=True,
text=True,
cwd=str(REPO_ROOT),
env=env,
timeout=30,
check=False,
)
class TestNativeTransportBinding(unittest.TestCase):
@@ -63,53 +39,35 @@ class TestNativeTransportBinding(unittest.TestCase):
mcp_daemon_guard.assert_sanctioned_mutation_runtime("offline_import")
msg = str(ctx.exception)
self.assertIn("#695", msg)
# FORCE disables pytest allowance; direct-import env is rejected first
# (AC1). Without ALLOW_DIRECT, env-alone also yields "not sufficient".
lowered = msg.lower()
self.assertTrue(
"direct" in lowered
or "not sufficient" in lowered
or "allow_direct" in lowered
or "gitea_allow_direct" in lowered,
msg,
)
self.assertIn("not sufficient", msg.lower() + " " + msg)
def test_direct_import_mark_rejected_outside_entrypoint(self):
mcp_daemon_guard.clear_native_runtime_for_tests()
os.environ[mcp_daemon_guard.FORCE_PROVENANCE_FAIL_ENV] = "1"
with self.assertRaises(mcp_daemon_guard.UnsanctionedRuntimeError) as ctx:
mcp_daemon_guard.mark_sanctioned_daemon()
self.assertIn("canonical", str(ctx.exception).lower())
self.assertIn("mcp_server.py", str(ctx.exception))
self.assertFalse(mcp_daemon_guard.is_native_mcp_transport())
def test_locally_generated_runtime_key_without_entrypoint_rejected(self):
"""Spoofing process-local fields via mark outside entrypoint fails."""
mcp_daemon_guard.clear_native_runtime_for_tests()
os.environ[mcp_daemon_guard.FORCE_PROVENANCE_FAIL_ENV] = "1"
# Even if a caller tries allow_test_bootstrap under force-unsanctioned
# pytest path is also forced off — only real entrypoint may mark.
with self.assertRaises(mcp_daemon_guard.UnsanctionedRuntimeError):
mcp_daemon_guard.mark_sanctioned_daemon()
mcp_daemon_guard.mark_sanctioned_daemon(allow_test_bootstrap=False)
def test_test_native_runtime_for_hermetic_tests_not_production(self):
def test_test_bootstrap_establishes_native_for_hermetic_tests(self):
mcp_daemon_guard.clear_native_runtime_for_tests()
st = mcp_daemon_guard.install_test_native_runtime()
st = mcp_daemon_guard.mark_sanctioned_daemon(allow_test_bootstrap=True)
self.assertTrue(st["native_mcp_transport"])
self.assertTrue(mcp_daemon_guard.is_native_mcp_transport())
self.assertFalse(mcp_daemon_guard.is_production_native_mcp_transport())
mcp_daemon_guard.assert_sanctioned_mutation_runtime("test-bootstrap")
fields = mcp_daemon_guard.mutation_provenance_fields()
self.assertEqual(fields["transport"], "test_native_mcp")
self.assertEqual(fields["transport"], "native_mcp")
self.assertTrue(fields["native_mcp_transport"])
self.assertFalse(fields["production_native_mcp_transport"])
self.assertIsNotNone(fields["native_token_fingerprint"])
with self.assertRaises(mcp_daemon_guard.UnsanctionedRuntimeError) as ctx:
mcp_daemon_guard.assert_production_mutation_runtime("prod-endpoint")
self.assertIn("Test-mode", str(ctx.exception))
def test_no_allow_test_bootstrap_parameter_on_mark(self):
import inspect
sig = inspect.signature(mcp_daemon_guard.mark_sanctioned_daemon)
self.assertNotIn("allow_test_bootstrap", sig.parameters)
def test_exposed_token_env_never_grants_native(self):
"""Raw / exposed token env vars must never reconstruct native transport."""
@@ -138,249 +96,6 @@ class TestNativeTransportBinding(unittest.TestCase):
os.environ.pop(key, None)
class TestAC9BypassRegressions(unittest.TestCase):
"""AC9: empirically reproduced offline bypasses must fail closed (#695)."""
def tearDown(self) -> None:
mcp_daemon_guard.clear_native_runtime_for_tests()
os.environ.pop(mcp_daemon_guard.FORCE_PROVENANCE_FAIL_ENV, None)
def test_allow_test_bootstrap_cannot_authorize_fresh_offline_interpreter(self):
"""Finding 1: former allow_test_bootstrap forge is gone and rejected offline."""
snippet = textwrap.dedent(
"""
import inspect
import mcp_daemon_guard as g
sig = inspect.signature(g.mark_sanctioned_daemon)
assert "allow_test_bootstrap" not in sig.parameters, "bootstrap flag must not exist"
try:
g.mark_sanctioned_daemon(allow_test_bootstrap=True)
except TypeError:
pass
else:
raise SystemExit("mark_sanctioned_daemon accepted allow_test_bootstrap")
try:
g.install_test_native_runtime()
except g.UnsanctionedRuntimeError as exc:
assert "pytest" in str(exc).lower() or "#695" in str(exc)
else:
raise SystemExit("install_test_native_runtime authorized offline interpreter")
assert g.is_native_mcp_transport() is False
try:
g.assert_sanctioned_mutation_runtime("offline-bootstrap")
except g.UnsanctionedRuntimeError:
pass
else:
raise SystemExit("mutation runtime authorized after offline bootstrap attempt")
print("OK")
"""
)
proc = _run_offline_snippet(snippet)
self.assertEqual(proc.returncode, 0, proc.stdout + proc.stderr)
self.assertIn("OK", proc.stdout)
def test_renamed_runner_named_mcp_server_py_rejected(self):
"""Finding 2: basename-only entrypoint trust is insufficient."""
with tempfile.TemporaryDirectory() as tmp:
attacker = Path(tmp) / "mcp_server.py"
attacker.write_text(
textwrap.dedent(
"""
import mcp_daemon_guard as g
try:
g.mark_sanctioned_daemon()
except g.UnsanctionedRuntimeError as exc:
print("REJECTED:" + str(exc))
raise SystemExit(0)
print("AUTHORIZED")
raise SystemExit(1)
"""
),
encoding="utf-8",
)
env = os.environ.copy()
env.pop("PYTEST_CURRENT_TEST", None)
env["PYTHONPATH"] = str(REPO_ROOT) + os.pathsep + env.get("PYTHONPATH", "")
proc = subprocess.run(
[sys.executable, str(attacker)],
capture_output=True,
text=True,
cwd=tmp,
env=env,
timeout=30,
check=False,
)
self.assertEqual(proc.returncode, 0, proc.stdout + proc.stderr)
self.assertIn("REJECTED:", proc.stdout)
self.assertIn("canonical", proc.stdout.lower())
self.assertNotIn("AUTHORIZED", proc.stdout)
def test_direct_launch_import_canonical_entrypoint_without_transport_rejected(self):
"""Merely importing/launching real entrypoint offline must not authorize."""
snippet = textwrap.dedent(
f"""
import importlib.util
import mcp_daemon_guard as g
# Simulate claim-only phase (no transport bind).
g.clear_native_runtime_for_tests()
# Direct mark from non-entrypoint must fail.
try:
g.mark_sanctioned_daemon()
except g.UnsanctionedRuntimeError:
pass
assert g.is_native_mcp_transport() is False
# Even if someone forges entrypoint_claimed without transport bind:
g._NATIVE_RUNTIME = {{
"token": "x" * 64,
"token_fingerprint": "deadbeefdeadbeef",
"pid": __import__("os").getpid(),
"started_at": 0,
"entrypoint": "mcp_server",
"entrypoint_path": {str(REPO_ROOT / "mcp_server.py")!r},
"phase": "entrypoint_claimed",
"transport": None,
"mode": "production",
}}
assert g.is_native_mcp_transport() is False
try:
g.assert_sanctioned_mutation_runtime("import-only")
except g.UnsanctionedRuntimeError as exc:
assert "transport" in str(exc).lower() or "entrypoint" in str(exc).lower() or "#695" in str(exc)
else:
raise SystemExit("import-only claim authorized mutation")
print("OK")
"""
)
proc = _run_offline_snippet(snippet)
self.assertEqual(proc.returncode, 0, proc.stdout + proc.stderr)
self.assertIn("OK", proc.stdout)
def test_spoofed_pytest_env_stack_path_rejected(self):
"""Spoofed pytest/env/call-stack/path evidence must not authorize offline."""
snippet = textwrap.dedent(
f"""
import os
import mcp_daemon_guard as g
os.environ["PYTEST_CURRENT_TEST"] = "spoofed::test"
os.environ[g.SANCTIONED_DAEMON_ENV] = "1"
os.environ[g.ALLOW_DIRECT_IMPORT_ENV] = "1"
# Fresh interpreter has no pytest module; PYTEST_CURRENT_TEST alone
# might still trip is_pytest_runtime — force-unsanctioned is not set.
# But install_test_native_runtime requires real pytest path; if
# PYTEST_CURRENT_TEST alone grants is_pytest_runtime, production
# mutation still requires production transport outside true pytest.
if g.is_pytest_runtime():
# Env-only pytest spoof: test install may succeed, but production
# mutation gate must still reject test mode.
g.install_test_native_runtime()
assert g.is_production_native_mcp_transport() is False
try:
g.assert_production_mutation_runtime("spoofed-pytest")
except g.UnsanctionedRuntimeError:
pass
else:
raise SystemExit("production mutation accepted test-mode under spoofed pytest env")
else:
try:
g.install_test_native_runtime()
except g.UnsanctionedRuntimeError:
pass
else:
raise SystemExit("test install without pytest evidence")
# Basename path spoof via inspect is covered elsewhere; env alone:
g.clear_native_runtime_for_tests()
os.environ.pop("PYTEST_CURRENT_TEST", None)
assert g.is_native_mcp_transport() is False
try:
g.assert_sanctioned_mutation_runtime("env-path-spoof")
except g.UnsanctionedRuntimeError:
pass
else:
raise SystemExit("env/path spoof authorized mutation")
print("OK")
"""
)
proc = _run_offline_snippet(snippet)
self.assertEqual(proc.returncode, 0, proc.stdout + proc.stderr)
self.assertIn("OK", proc.stdout)
def test_test_bootstrap_cannot_reach_production_mutation_endpoints(self):
"""Under pytest, test-mode runtime cannot satisfy production mutation gate."""
mcp_daemon_guard.clear_native_runtime_for_tests()
mcp_daemon_guard.install_test_native_runtime()
with self.assertRaises(mcp_daemon_guard.UnsanctionedRuntimeError) as ctx:
mcp_daemon_guard.assert_production_mutation_runtime(
"gitea_quarantine_contaminated_review"
)
msg = str(ctx.exception)
self.assertIn("Test-mode", msg)
self.assertIn("#695", msg)
# Offline: install_test_native_runtime must not authorize production mutations.
snippet = textwrap.dedent(
"""
import mcp_daemon_guard as g
try:
g.install_test_native_runtime()
except g.UnsanctionedRuntimeError:
pass
assert g.is_production_native_mcp_transport() is False
try:
g.assert_production_mutation_runtime("gitea_quarantine_contaminated_review")
except g.UnsanctionedRuntimeError:
pass
else:
raise SystemExit("production mutation authorized offline")
try:
g.assert_sanctioned_mutation_runtime("gitea_mutation")
except g.UnsanctionedRuntimeError:
pass
else:
raise SystemExit("sanctioned mutation authorized offline")
print("OK")
"""
)
proc = _run_offline_snippet(snippet)
self.assertEqual(proc.returncode, 0, proc.stdout + proc.stderr)
self.assertIn("OK", proc.stdout)
def test_legitimate_native_transport_bind_succeeds(self):
"""Canonical entrypoint path + stdio bind establishes production native."""
mcp_daemon_guard.clear_native_runtime_for_tests()
# Simulate production path under force-unsanctioned (no pytest allowance)
# by calling internal claim/bind with patched caller path.
canonical = str((REPO_ROOT / "mcp_server.py").resolve())
def _fake_caller():
return canonical
with patch.object(
mcp_daemon_guard, "_caller_official_entrypoint_path", side_effect=_fake_caller
):
# Force non-pytest path for mark/bind logic.
with patch.object(mcp_daemon_guard, "is_pytest_runtime", return_value=False):
st1 = mcp_daemon_guard.mark_sanctioned_daemon()
self.assertFalse(st1["native_mcp_transport"])
self.assertEqual(st1["phase"], "entrypoint_claimed")
st2 = mcp_daemon_guard.bind_native_mcp_transport(transport="stdio")
self.assertTrue(st2["native_mcp_transport"])
self.assertTrue(st2["production_native_mcp_transport"])
self.assertEqual(st2["transport"], "stdio")
self.assertEqual(st2["mode"], "production")
mcp_daemon_guard.assert_sanctioned_mutation_runtime("native-ide")
mcp_daemon_guard.assert_production_mutation_runtime("native-ide")
fields = mcp_daemon_guard.mutation_provenance_fields()
self.assertEqual(fields["transport"], "native_mcp")
self.assertTrue(fields["production_native_mcp_transport"])
def test_canonical_entrypoint_paths_are_resolved_absolute(self):
paths = mcp_daemon_guard.canonical_entrypoint_paths()
self.assertTrue(any(p.endswith("mcp_server.py") for p in paths))
for p in paths:
self.assertTrue(os.path.isabs(p), p)
self.assertEqual(p, str(Path(p).resolve()))
class TestQuarantineWriteNativeOnly(unittest.TestCase):
def setUp(self) -> None:
self._tmp = tempfile.TemporaryDirectory()
@@ -418,12 +133,7 @@ class TestQuarantineWriteNativeOnly(unittest.TestCase):
forensic_comment_ids=[10883, 10886],
)
with self.assertRaises(mcp_daemon_guard.UnsanctionedRuntimeError):
# Force non-pytest and non-native for write path.
with patch.object(mcp_daemon_guard, "is_pytest_runtime", return_value=False):
with patch.object(
mcp_daemon_guard, "is_native_mcp_transport", return_value=False
):
review_quarantine.write_quarantine_record(record)
review_quarantine.write_quarantine_record(record)
def test_confirmation_must_match_exactly(self):
assessment = review_quarantine.assess_quarantine_write(
@@ -460,7 +170,7 @@ class TestQuarantineWriteNativeOnly(unittest.TestCase):
"review_quarantine.mcp_session_state.default_state_dir",
return_value=self._tmp.name,
):
mcp_daemon_guard.install_test_native_runtime()
mcp_daemon_guard.mark_sanctioned_daemon(allow_test_bootstrap=True)
record = review_quarantine.build_quarantine_record(
remote="prgs",
org="org",
@@ -595,7 +305,7 @@ class TestFeedbackQuarantineIntegration(unittest.TestCase):
self._tmp = tempfile.TemporaryDirectory()
self.addCleanup(self._tmp.cleanup)
mcp_daemon_guard.clear_native_runtime_for_tests()
mcp_daemon_guard.install_test_native_runtime()
mcp_daemon_guard.mark_sanctioned_daemon(allow_test_bootstrap=True)
def tearDown(self) -> None:
mcp_daemon_guard.clear_native_runtime_for_tests()
@@ -686,163 +396,5 @@ class TestDocsStopAfterNativeFailure(unittest.TestCase):
self.assertIn("GITEA_ALLOW_DIRECT_MCP_IMPORT", doc)
class TestPR701DirectImportSessionOverrideSequence(unittest.TestCase):
"""AC9 regression: exact PR #701 contaminated-approval sequence must fail closed.
Observed attack:
- GITEA_ALLOW_DIRECT_MCP_IMPORT=1
- import mutation tools from gitea_mcp_server
- GITEA_MCP_SESSION_STATE_DIR=.mcp_session_701 (cross-PR lock evasion)
- mark_final + submit_pr_review
"""
def tearDown(self) -> None:
mcp_daemon_guard.clear_native_runtime_for_tests()
os.environ.pop(mcp_daemon_guard.FORCE_PROVENANCE_FAIL_ENV, None)
os.environ.pop(mcp_daemon_guard.SANCTIONED_DAEMON_ENV, None)
os.environ.pop(mcp_daemon_guard.ALLOW_DIRECT_IMPORT_ENV, None)
os.environ.pop(mcp_daemon_guard.SESSION_STATE_DIR_ENV, None)
def test_offline_run_submit_sequence_fails_closed(self):
"""Fresh interpreter: direct import + state-dir override cannot mark/submit."""
with tempfile.TemporaryDirectory() as tmp:
redirect = str(Path(tmp) / ".mcp_session_701")
snippet = textwrap.dedent(
f"""
import os
import sys
os.environ["GITEA_ALLOW_DIRECT_MCP_IMPORT"] = "1"
os.environ["GITEA_MCP_SESSION_STATE_DIR"] = {redirect!r}
os.environ["GITEA_MCP_PROFILE"] = "prgs-reviewer"
# No pytest modules in this subprocess.
import mcp_daemon_guard as g
assert g.is_native_mcp_transport() is False
assert g.is_production_native_mcp_transport() is False
try:
g.assert_sanctioned_mutation_runtime("run_submit_mark")
except g.UnsanctionedRuntimeError as exc:
msg = str(exc)
assert "GITEA_ALLOW_DIRECT_MCP_IMPORT" in msg or "#695" in msg
else:
raise SystemExit("direct-import env authorized mutation runtime")
try:
g.mark_sanctioned_daemon()
except g.UnsanctionedRuntimeError:
pass
else:
raise SystemExit("mark_sanctioned_daemon authorized offline import")
# Simulate decision-lock write into redirected dir only — must not
# establish native authority.
import mcp_session_state as ss
wrote = ss.save_state(
kind=ss.KIND_DECISION_LOCK,
payload={{
"final_review_decision_ready": True,
"ready_pr_number": 701,
"ready_action": "approve",
"ready_expected_head_sha": "6b675f5c834b41f9d74e8a54294ff44dddf28ae4",
"session_profile": "prgs-reviewer",
"session_profile_lock": "prgs-reviewer",
"remote": "prgs",
}},
profile_identity="prgs-reviewer",
state_dir={redirect!r},
)
assert wrote is not None
assert g.is_native_mcp_transport() is False
try:
g.assert_no_direct_import_bypass("gitea_submit_pr_review")
except g.UnsanctionedRuntimeError:
pass
else:
raise SystemExit("direct-import bypass accepted for submit")
print("OK")
"""
)
proc = _run_offline_snippet(snippet)
self.assertEqual(proc.returncode, 0, proc.stdout + proc.stderr)
self.assertIn("OK", proc.stdout)
def test_session_state_dir_pin_ignores_post_bind_redirect(self):
"""AC2: after production bind, env STATE_DIR override is ignored."""
mcp_daemon_guard.clear_native_runtime_for_tests()
import mcp_session_state
with tempfile.TemporaryDirectory() as tmp:
legitimate = str(Path(tmp) / "legitimate-state")
rogue = str(Path(tmp) / ".mcp_session_701")
os.makedirs(legitimate, mode=0o700, exist_ok=True)
os.makedirs(rogue, mode=0o700, exist_ok=True)
os.environ[mcp_daemon_guard.SESSION_STATE_DIR_ENV] = legitimate
canonical = str((REPO_ROOT / "mcp_server.py").resolve())
def _fake_caller():
return canonical
with patch.object(
mcp_daemon_guard,
"_caller_official_entrypoint_path",
side_effect=_fake_caller,
):
with patch.object(
mcp_daemon_guard, "is_pytest_runtime", return_value=False
):
mcp_daemon_guard.mark_sanctioned_daemon()
mcp_daemon_guard.bind_native_mcp_transport(transport="stdio")
pinned = mcp_daemon_guard.pinned_session_state_dir()
self.assertEqual(pinned, str(Path(legitimate).resolve()))
# Attacker redirects env after bind (PR #701).
os.environ[mcp_daemon_guard.SESSION_STATE_DIR_ENV] = rogue
self.assertEqual(
mcp_daemon_guard.pinned_session_state_dir(),
str(Path(legitimate).resolve()),
)
self.assertEqual(
mcp_session_state.default_state_dir(),
str(Path(legitimate).resolve()),
)
self.assertNotEqual(
mcp_session_state.default_state_dir(),
str(Path(rogue).resolve()),
)
# Unpinned env view still sees rogue (diagnostics only).
unpinned = mcp_session_state.env_session_state_dir_unpinned()
self.assertTrue(
unpinned == rogue
or Path(unpinned).resolve() == Path(rogue).resolve(),
unpinned,
)
def test_direct_import_env_does_not_authorize_under_force_unsanctioned(self):
mcp_daemon_guard.clear_native_runtime_for_tests()
os.environ[mcp_daemon_guard.ALLOW_DIRECT_IMPORT_ENV] = "1"
os.environ[mcp_daemon_guard.FORCE_PROVENANCE_FAIL_ENV] = "1"
self.assertTrue(mcp_daemon_guard.direct_import_env_enabled())
# Under pytest, assert_no_direct_import_bypass is a no-op; FORCE path
# still blocks is_native / assert_sanctioned via force-unsanctioned.
self.assertFalse(mcp_daemon_guard.is_native_mcp_transport())
with self.assertRaises(mcp_daemon_guard.UnsanctionedRuntimeError):
mcp_daemon_guard.assert_sanctioned_mutation_runtime("direct-import")
def test_quarantine_voids_merge_approval_for_contaminated_review(self):
"""AC6AC8: quarantined APPROVED does not satisfy merge approval head."""
entry = {
"verdict": "APPROVED",
"dismissed": False,
"reviewed_head_sha": "6b675f5c834b41f9d74e8a54294ff44dddf28ae4",
"review_id": 431,
"submitted_at": "2026-07-13T23:52:34Z",
"quarantined": True,
}
result = merge_approval_gate.assess_merge_approval_head(
current_head_sha="6b675f5c834b41f9d74e8a54294ff44dddf28ae4",
latest_by_reviewer={"sysadmin": entry},
quarantined_review_ids={431},
)
self.assertFalse(result["approval_at_current_head"])
self.assertEqual(result["quarantined_approvals_at_current_head"], 1)
self.assertIn("quarantined", (result["stale_approval_block_reason"] or ""))
if __name__ == "__main__":
unittest.main()
@@ -1,461 +0,0 @@
"""Regression tests for #698: final-report validator vs canonical schema.
Covers the original #698 lead plus the independent reproduction recorded
during the PR #703 formal review (issue #698 comment 11246):
1. non-dict ``action_log`` entries must fail structured, never crash;
2. the validator must not demand legacy fields the canonical schema forbids
(``Pinned reviewed head``, ``Scratch worktree used``, ``Worktree path``,
``Worktree dirty``, ``Mutations``, ``Next``);
3. a legitimately blocked report (``Candidate head SHA: none``, no formal
verdict) must not owe approval/merge live-head proofs;
4. canonical reviewer lease release must not be misclassified as post-merge
cleanup;
5. structured workflow-load and validation proof must be recognized;
6. review mutations are inferred only from authoritative evidence.
"""
from __future__ import annotations
import sys
import unittest
from pathlib import Path
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
import final_report_validator as frv # noqa: E402
import post_merge_cleanup_proof as pmcp # noqa: E402
import pr_work_lease as pwl # noqa: E402
import review_proofs as rp # noqa: E402
from review_final_report_schema import ( # noqa: E402
assess_review_final_report_schema,
)
REVIEWED_HEAD = "a" * 40
LIVE_HEAD = "a" * 40
def _pr703_style_report(
*,
decision: str = "request_changes",
cleanup_mutations: str = (
"released reviewer PR lease via gitea_release_reviewer_pr_lease "
"(terminal lease marker phase=released posted)"
),
) -> str:
"""Canonical-schema report modeled on the PR #703 formal review handoff."""
return f"""
Formal review completed with a REQUEST_CHANGES verdict submitted and read
back via the native review API.
## Controller Handoff
- Task: review-merge-pr
- Repo: Scaled-Tech-Consulting/Gitea-Tools
- Role: reviewer
- Identity: sysadmin / prgs-reviewer
- Active profile: prgs-reviewer
- Runtime context: neutral workspace binding
- Selected PR: 703
- Linked issue: #702 open
- Eligibility class: reviewable
- Queue ordering policy: oldest eligible first
- Inventory pagination proof: has_more=false, total_count=8
- Earlier PRs skipped: none
- Candidate head SHA: {REVIEWED_HEAD}
- Reviewed head SHA: {REVIEWED_HEAD}
- Target branch: master
- Target branch SHA: {"2" * 40}
- Already-landed gate: not landed
- Author-safety result: pass (author differs from reviewer)
- Prior request-changes state: none
- Review worktree used: true
- Review worktree path: branches/review-pr-703-independent
- Review worktree inside branches: true
- Review worktree HEAD state: detached at pinned head
- Review worktree dirty before validation: clean
- Review worktree dirty after validation: clean
- Baseline worktree used: false
- Baseline worktree path: none
- Files reviewed: 4
- Validation: focused 50 passed; related 94 passed; full 2665 passed, 6 skipped
- Official validation integrity status: intact
- Terminal review mutation: one REQUEST_CHANGES review submitted and read back
- Review decision: {decision}
- Merge preflight: not run
- Merge result: none
- Linked issue status: open (live fetch proof: gitea_view_issue)
- Main checkout branch: master
- Main checkout dirty state: clean
- Main checkout updated: false
- File edits by reviewer: none
- Worktree/index mutations: none
- Git ref mutations: git fetch prgs (recorded)
- MCP/Gitea mutations: review submission and lease comments only
- Review mutations: one formal REQUEST_CHANGES verdict
- Merge mutations: none
- Cleanup mutations: {cleanup_mutations}
- External-state mutations: none
- Read-only diagnostics: gitea_view_pr, gitea_get_pr_review_feedback
- Blockers: findings F1-F6 recorded on the PR thread
- Current status: review complete; author remediation required
- Safe next action: author addresses findings and pushes a new head
- Safety statement: no merge attempted; no self-review; no root-checkout edits
- Workflow-load helper result: workflow_hash=da045d1e1f1f boundary_status=clean
- Live head SHA before approval: {LIVE_HEAD}
- Pushes occurred during validation: no
"""
def _blocked_preflight_report() -> str:
"""Blocked-run report modeled on the #702 comment 11164 reproduction."""
return """
Fresh review preflight stopped before any worktree or validation work.
## Controller Handoff
- Task: review-merge-pr
- Repo: Scaled-Tech-Consulting/Gitea-Tools
- Role: reviewer
- Identity: sysadmin / prgs-reviewer
- Active profile: prgs-reviewer
- Runtime context: stale workspace binding detected
- Selected PR: 701
- Linked issue: #699 open
- Eligibility class: blocked-before-validation
- Queue ordering policy: oldest eligible first
- Inventory pagination proof: has_more=false, total_count=8
- Earlier PRs skipped: none
- Candidate head SHA: none
- Reviewed head SHA: none
- Target branch: master
- Target branch SHA: none
- Already-landed gate: not run
- Author-safety result: not run
- Prior request-changes state: none
- Review worktree used: false
- Review worktree path: none
- Review worktree inside branches: not applicable
- Review worktree HEAD state: not applicable
- Review worktree dirty before validation: not applicable
- Review worktree dirty after validation: not applicable
- Baseline worktree used: false
- Baseline worktree path: none
- Files reviewed: 0
- Validation: not run
- Official validation integrity status: not applicable
- Terminal review mutation: none
- Review decision: none
- Merge preflight: not run
- Merge result: none
- Linked issue status: open (live fetch proof: gitea_view_issue)
- Main checkout branch: master
- Main checkout dirty state: clean
- Main checkout updated: false
- File edits by reviewer: none
- Worktree/index mutations: none
- Git ref mutations: none
- MCP/Gitea mutations: none
- Review mutations: none
- Merge mutations: none
- Cleanup mutations: none
- External-state mutations: none
- Read-only diagnostics: gitea_view_pr, gitea_get_runtime_context
- Blockers: runtime bound to a foreign task worktree; mutation prohibited
- Current status: stopped before validation began
- Next actor: operator
- Next action: repair the runtime workspace binding, then rerun the full
review workflow in a fresh reviewer session
- Next prompt: Act as REVIEWER for PR 701 after the operator repairs the
runtime binding; acquire the lease before any validation.
- Safe next action: operator repairs runtime binding, then a fresh reviewer
reruns the full workflow
- Safety statement: no lease acquired; no verdict recorded; no source edits
- Workflow-load helper result: workflow_hash=da045d1e1f1f boundary_status=clean
"""
class TestActionLogRobustness(unittest.TestCase):
"""#698 original lead: non-dict action_log must not crash validation."""
MALFORMED = [
"git fetch prgs",
42,
None,
{"action": "edit", "path": "x.py", "performed": True, "tracked": True},
]
def test_assess_final_report_validator_survives_malformed_entries(self):
result = frv.assess_final_report_validator(
_pr703_style_report(),
"review_pr",
action_log=self.MALFORMED,
)
self.assertIsInstance(result, dict)
rule_ids = {f["rule_id"] for f in result["findings"]}
self.assertIn("shared.action_log_malformed", rule_ids)
def test_malformed_entry_errors_are_sanitized(self):
_entries, findings = frv.sanitize_action_log(["secret-token-abc123"])
self.assertEqual(len(findings), 1)
reason = findings[0]["reason"]
self.assertNotIn("secret-token-abc123", reason)
self.assertIn("str", reason)
self.assertIn("entry 0", reason)
def test_non_list_action_log_is_reported_not_raised(self):
entries, findings = frv.sanitize_action_log("not-a-list")
self.assertEqual(entries, [])
self.assertEqual(len(findings), 1)
self.assertIn("not a list", findings[0]["reason"])
def test_performed_file_mutations_skips_non_dict_entries(self):
performed = rp._performed_file_mutations(
["oops", {"action": "edited", "path": "a.py"}]
)
self.assertEqual(len(performed), 1)
self.assertEqual(performed[0]["path"], "a.py")
def test_schema_entrypoint_survives_string_only_log(self):
result = assess_review_final_report_schema(
_pr703_style_report(),
action_log=["just a string", "another string"],
)
self.assertIsInstance(result, dict)
class TestLegacyFieldRequirementsRemoved(unittest.TestCase):
"""#698: prohibited legacy fields must not be REQUIRED of reports."""
PROHIBITED = (
"Pinned reviewed head",
"Scratch worktree used",
"Worktree path",
"Worktree dirty",
"Workspace mutations",
"Mutations",
"Next",
"Issue/PR",
"Branch/SHA",
"Files changed",
)
def test_review_role_field_table_has_no_prohibited_requirements(self):
names = [name for name, _ in rp.HANDOFF_ROLE_FIELDS["review"]]
for prohibited in ("Pinned reviewed head", "Scratch worktree used",
"Worktree path", "Worktree dirty"):
self.assertNotIn(prohibited, names)
def test_merger_role_field_table_has_no_pinned_reviewed_head(self):
names = [name for name, _ in rp.HANDOFF_ROLE_FIELDS["merger"]]
self.assertNotIn("Pinned reviewed head", names)
def test_canonical_report_missing_fields_never_include_prohibited(self):
result = rp.assess_controller_handoff(
_pr703_style_report(), role="review"
)
for prohibited in self.PROHIBITED:
self.assertNotIn(prohibited, result.get("missing_fields") or [])
def test_canonical_pr703_report_satisfies_required_fields(self):
result = rp.assess_controller_handoff(
_pr703_style_report(), role="review"
)
self.assertEqual(result.get("missing_fields") or [], [])
self.assertEqual(result.get("verdict"), "complete")
class TestBlockedReportAccepted(unittest.TestCase):
"""#698: blocked run with no reviewed head / verdict is legitimate."""
def test_stale_head_proof_waived_before_validation(self):
result = pwl.assess_reviewer_stale_head_final_report(
_blocked_preflight_report()
)
self.assertTrue(result["proven"])
self.assertEqual(result.get("phase"), "blocked_before_validation")
def test_blocked_report_passes_schema_validation(self):
result = assess_review_final_report_schema(_blocked_preflight_report())
blocking = [
f for f in result["findings"] if f["severity"] == "block"
]
self.assertEqual(blocking, [], blocking)
def test_verdict_phase_still_demands_approval_head_proof(self):
report = _blocked_preflight_report().replace(
"- Review decision: none",
"- Review decision: approve",
).replace(
"- Candidate head SHA: none",
f"- Candidate head SHA: {REVIEWED_HEAD}",
)
result = pwl.assess_reviewer_stale_head_final_report(report)
self.assertFalse(result["proven"])
joined = " ".join(result["reasons"])
self.assertIn("before approval", joined)
def test_merge_phase_still_demands_merge_head_proof(self):
report = _pr703_style_report().replace(
"- Merge result: none",
"- Merge result: merged",
)
result = pwl.assess_reviewer_stale_head_final_report(report)
self.assertFalse(result["proven"])
self.assertIn(
"final live head SHA before merge not stated",
result["reasons"],
)
def test_validation_phase_demands_push_disclosure(self):
report = _pr703_style_report().replace(
"- Pushes occurred during validation: no\n", ""
)
result = pwl.assess_reviewer_stale_head_final_report(report)
self.assertFalse(result["proven"])
self.assertIn(
"whether push occurred during validation not stated",
result["reasons"],
)
class TestLeaseReleaseVsPostMergeCleanup(unittest.TestCase):
"""#698 (PR #703 review reproduction): lease release is not cleanup."""
def test_lease_release_cleanup_mutations_do_not_demand_checklist(self):
result = pmcp.assess_post_merge_cleanup_proof(_pr703_style_report())
self.assertFalse(result["block"], result["reasons"])
def test_release_tool_name_alone_is_recognized(self):
report = _pr703_style_report(
cleanup_mutations="gitea_release_reviewer_pr_lease comment 11244"
)
result = pmcp.assess_post_merge_cleanup_proof(report)
self.assertFalse(result["block"], result["reasons"])
def test_substantive_non_lease_cleanup_still_demands_checklist(self):
report = _pr703_style_report(
cleanup_mutations="deleted stale scratch directory manually"
)
result = pmcp.assess_post_merge_cleanup_proof(report)
self.assertTrue(result["block"])
def test_remote_branch_delete_claims_still_demand_full_proof(self):
report = _pr703_style_report(
cleanup_mutations="gitea_delete_branch removed the remote branch"
)
result = pmcp.assess_post_merge_cleanup_proof(report)
self.assertTrue(result["block"])
self.assertTrue(
any("remote branch deletion missing" in r for r in result["reasons"])
)
def test_full_schema_run_accepts_lease_release_report(self):
result = assess_review_final_report_schema(_pr703_style_report())
lease_cleanup_blocks = [
f for f in result["findings"]
if f["rule_id"] == "reviewer.post_merge_cleanup_proof"
]
self.assertEqual(lease_cleanup_blocks, [], lease_cleanup_blocks)
class TestStructuredProofRecognition(unittest.TestCase):
"""#698: structured workflow-load and validation proof must be accepted."""
def test_key_value_workflow_proof_recognized(self):
findings = frv._rule_reviewer_workflow_load_boundary(
_pr703_style_report()
)
self.assertEqual(findings, [], findings)
def test_colon_form_workflow_proof_still_recognized(self):
report = _pr703_style_report().replace(
"- Workflow-load helper result: workflow_hash=da045d1e1f1f "
"boundary_status=clean",
"- Workflow-load helper result: workflow_hash: da045d1e1f1f, "
"boundary_status: clean",
)
findings = frv._rule_reviewer_workflow_load_boundary(report)
self.assertEqual(findings, [], findings)
def test_incomplete_structured_proof_still_blocks(self):
report = _pr703_style_report().replace(
"workflow_hash=da045d1e1f1f boundary_status=clean",
"workflow_hash=da045d1e1f1f",
)
findings = frv._rule_reviewer_workflow_load_boundary(report)
self.assertTrue(findings)
self.assertIn("boundary_status", findings[0]["reason"])
def test_validation_counts_accepted_as_pass_proof(self):
# "Validation: focused 50 passed; ..." must satisfy the reviewed-head
# validation-proof rule (PR #703 reproduction).
result = assess_review_final_report_schema(_pr703_style_report())
head_blocks = [
f for f in result["findings"]
if f["rule_id"] == "reviewer.reviewed_head_without_validation"
]
self.assertEqual(head_blocks, [], head_blocks)
class TestAuthoritativeMutationInference(unittest.TestCase):
"""#698: review mutations inferred only from authoritative evidence."""
def test_read_only_entries_do_not_imply_mutations(self):
report = "## Controller Handoff\n- Mutations: none\n"
findings = frv._rule_reviewer_vague_mutations_none(
report,
action_log=[
{"action": "gitea_view_pr"},
{"action": "gitea_get_pr_review_feedback", "performed": False},
],
)
self.assertEqual(findings, [], findings)
def test_performed_mutation_still_blocks_vague_none(self):
report = "## Controller Handoff\n- Mutations: none\n"
findings = frv._rule_reviewer_vague_mutations_none(
report,
action_log=[{"action": "edit", "path": "a.py", "performed": True}],
)
self.assertTrue(findings)
def test_gated_rejection_is_not_a_mutation(self):
report = "## Controller Handoff\n- Mutations: none\n"
findings = frv._rule_reviewer_vague_mutations_none(
report,
action_log=[
{"action": "edit", "path": "a.py", "performed": True,
"gated_rejected": True},
],
)
self.assertEqual(findings, [], findings)
class TestValidatorRuleErrorContainment(unittest.TestCase):
"""#698: a defective rule fails closed with a sanitized error."""
def test_rule_exception_becomes_sanitized_block_finding(self):
def _boom(report_text):
raise ValueError("raw secret detail that must not leak")
original = frv._RULES_BY_TASK["review_pr"]
frv._RULES_BY_TASK["review_pr"] = [_boom]
try:
result = frv.assess_final_report_validator(
"report body", "review_pr"
)
finally:
frv._RULES_BY_TASK["review_pr"] = original
self.assertTrue(result["blocked"])
finding = next(
f for f in result["findings"]
if f["rule_id"] == "shared.validator_rule_error"
)
self.assertNotIn("raw secret detail", finding["reason"])
self.assertIn("ValueError", finding["reason"])
self.assertIn("_boom", finding["reason"])
if __name__ == "__main__":
unittest.main()
File diff suppressed because it is too large Load Diff
@@ -1,499 +0,0 @@
"""#720: Expired old-head KIND_DECISION_LOCK must not block fresh review.
Reproduction shape (PR #616):
* REQUEST_CHANGES terminal at head A
* open PR advanced to head B
* durable decision lock age > default 4h TTL
* fresh_review_on_current_head_allowed is true but unreachable under TTL-first reject
* mark_final fails with "session state expired after 4h"
* assessment may report "no lock" while the file remains on disk
Security invariants preserved:
* same-head second terminal remains fail-closed (#332/#620)
* REQUEST_CHANGES is never treated as approval
* historical mutations remain on the ledger
* merged/closed moot cleanup still allowed (#594)
* irrecoverable provenance still requires #709 authorization
* ordinary profiles do not gain gitea.decision_lock.irrecoverable_recovery
"""
from __future__ import annotations
import os
import tempfile
import unittest
from datetime import datetime, timedelta, timezone
from pathlib import Path
from unittest.mock import patch
import sys
ROOT = str(Path(__file__).resolve().parent.parent)
if ROOT not in sys.path:
sys.path.insert(0, ROOT)
import mcp_session_state as ss
import mcp_server
import stale_review_decision_lock as srdl
import task_capability_map
HEAD_A = "a0fffae576673ba7df7456b32e6aec916581bdfb"
HEAD_B = "a6a2243aad9c3e385fc70509f4941a0f0ec33162"
HEAD_SAME = HEAD_A
RC_616_A = {
"pr_number": 616,
"action": "request_changes",
"review_id": 443,
"review_state": "request_changes",
"head_sha": HEAD_A,
}
def _lock(mutations=None, **kwargs):
base = {
"task": "review_pr",
"kind": ss.KIND_DECISION_LOCK,
"remote": "prgs",
"org": "Scaled-Tech-Consulting",
"repo": "Gitea-Tools",
"session_pid": os.getpid(),
"session_profile": "prgs-reviewer",
"session_profile_lock": "prgs-reviewer",
"profile_identity": "prgs-reviewer",
"final_review_decision_ready": False,
"ready_pr_number": kwargs.get("ready_pr"),
"ready_action": kwargs.get("ready_action"),
"ready_expected_head_sha": kwargs.get("ready_head"),
"ready_remote": "prgs" if kwargs.get("ready_pr") else None,
"ready_org": "Scaled-Tech-Consulting" if kwargs.get("ready_pr") else None,
"ready_repo": "Gitea-Tools" if kwargs.get("ready_pr") else None,
"live_mutations": list(mutations or []),
"correction_authorized": False,
"correction_reason": None,
}
return base
def _open_pr(pr_number=616, head=HEAD_B, merged=False, closed=False):
state = "closed" if closed or merged else "open"
return {
"number": pr_number,
"state": state,
"merged": merged,
"merged_at": "2026-07-16T12:00:00Z" if merged else None,
"merge_commit_sha": "m" * 40 if merged else None,
"head": {"sha": head},
}
def _no_lease():
return {"block": False, "reasons": [], "mutation_allowed": True}
def _feedback(blocking=False, stale=True):
return {
"success": True,
"has_blocking_change_requests": blocking,
"review_feedback_stale": stale,
"current_head_sha": HEAD_B,
}
def _age_lock_payload(lock: dict, hours: float = 5.0) -> dict:
"""Rewrite timestamps to *hours* ago (simulates durable age without touching prod)."""
aged = (datetime.now(timezone.utc) - timedelta(hours=hours)).isoformat().replace(
"+00:00", "Z"
)
out = dict(lock)
out["recorded_at"] = aged
out["updated_at"] = aged
return out
class TestIssue720ExpiredDecisionLockLifecycle(unittest.TestCase):
def setUp(self):
self._tmp = tempfile.TemporaryDirectory()
self.env = patch.dict(
os.environ,
{
ss.STATE_DIR_ENV: self._tmp.name,
ss.SESSION_PROFILE_LOCK_ENV: "prgs-reviewer",
"GITEA_MCP_PROFILE": "prgs-reviewer",
"GITEA_PROFILE_NAME": "prgs-reviewer",
},
clear=False,
)
self.env.start()
mcp_server._REVIEW_DECISION_LOCK = None
import review_workflow_load
review_workflow_load.clear_review_workflow_load()
review_workflow_load.record_review_workflow_load(mcp_server.PROJECT_ROOT)
mcp_server.gitea_load_review_workflow()
def tearDown(self):
mcp_server._REVIEW_DECISION_LOCK = None
import review_workflow_load
review_workflow_load.clear_review_workflow_load()
self.env.stop()
self._tmp.cleanup()
def _persist_aged_lock(self, mutations, hours: float = 5.0, **kwargs):
"""Write decision lock aged > TTL to the temp durable store (test-only)."""
payload = _age_lock_payload(_lock(mutations, **kwargs), hours=hours)
saved = ss.save_state(
kind=ss.KIND_DECISION_LOCK,
payload=payload,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
profile_identity="prgs-reviewer",
)
# save_state refreshes updated_at; re-age the on-disk envelope for TTL tests.
path = ss.state_file_path(
kind=ss.KIND_DECISION_LOCK,
profile_identity="prgs-reviewer",
state_dir=self._tmp.name,
)
aged = (datetime.now(timezone.utc) - timedelta(hours=hours)).isoformat().replace(
"+00:00", "Z"
)
import json
with open(path, encoding="utf-8") as fh:
envelope = json.load(fh)
envelope["recorded_at"] = aged
envelope["updated_at"] = aged
if isinstance(envelope.get("payload"), dict):
envelope["payload"]["recorded_at"] = aged
envelope["payload"]["updated_at"] = aged
with open(path, "w", encoding="utf-8") as fh:
json.dump(envelope, fh, indent=2, sort_keys=True)
fh.write("\n")
# Drop memory so subsequent loads hit durable store.
mcp_server._REVIEW_DECISION_LOCK = None
return saved
def _mark(self, pr, action, head, feedback=None):
with patch("mcp_server._list_pr_lease_comments", return_value=[]), patch(
"mcp_server._pr_work_lease_reviewer_block", return_value=_no_lease()
), patch.object(
mcp_server,
"gitea_get_pr_review_feedback",
return_value=feedback or _feedback(blocking=False, stale=True),
), patch.object(
mcp_server,
"gitea_check_pr_eligibility",
return_value={"eligible": True, "head_sha": head},
), patch.object(
mcp_server.mcp_daemon_guard,
"assert_sanctioned_mutation_runtime",
return_value=None,
), patch.object(
mcp_server.mcp_daemon_guard,
"assert_no_direct_import_bypass",
return_value=None,
):
return mcp_server.gitea_mark_final_review_decision(
pr_number=pr,
action=action,
expected_head_sha=head,
remote="prgs",
org="Scaled-Tech-Consulting",
repo="Gitea-Tools",
)
# --- AC regression matrix ---
def test_under_four_hours_fresh_review_at_b_allowed(self):
self._persist_aged_lock(
[RC_616_A],
hours=1.0,
ready_pr=616,
ready_head=HEAD_A,
ready_action="request_changes",
)
res = self._mark(616, "approve", HEAD_B)
self.assertTrue(res.get("marked_ready"), res)
def test_over_four_hours_fresh_review_at_b_still_allowed(self):
"""Primary #720 defect: age > TTL must not block head-B mark_final."""
self._persist_aged_lock(
[RC_616_A],
hours=5.0,
ready_pr=616,
ready_head=HEAD_A,
ready_action="request_changes",
)
# Prove durable load is possible for recovery-critical decision locks.
loaded = ss.load_state(
kind=ss.KIND_DECISION_LOCK,
profile_identity="prgs-reviewer",
)
self.assertIsNotNone(
loaded,
"expired KIND_DECISION_LOCK must remain loadable (not generic TTL cache)",
)
res = self._mark(616, "approve", HEAD_B)
self.assertTrue(
res.get("marked_ready"),
f"expected mark_ready on head B after >4h; got {res}",
)
reasons = " ".join(res.get("reasons") or [])
self.assertNotIn("session state expired", reasons)
def test_historical_review_at_a_preserved_and_stale(self):
self._persist_aged_lock(
[RC_616_A],
hours=5.0,
ready_pr=616,
ready_head=HEAD_A,
ready_action="request_changes",
)
res = self._mark(616, "approve", HEAD_B)
self.assertTrue(res.get("marked_ready"), res)
lock = mcp_server._load_review_decision_lock()
self.assertIsNotNone(lock)
hist = [m for m in lock["live_mutations"] if m.get("review_id") == 443]
self.assertEqual(len(hist), 1)
self.assertEqual(hist[0]["head_sha"], HEAD_A)
self.assertEqual(hist[0]["action"], "request_changes")
a = srdl.assess_stale_review_decision_lock(
lock, pr_live=_open_pr(616, HEAD_B)
)
self.assertTrue(a["stale_by_head"])
self.assertTrue(a["fresh_review_on_current_head_allowed"])
self.assertEqual(a["locked_head_sha"], HEAD_A)
def test_no_reuse_approval_from_head_a(self):
"""REQUEST_CHANGES at A is never an approval credential for B."""
self._persist_aged_lock([RC_616_A], hours=5.0)
lock = ss.load_state(
kind=ss.KIND_DECISION_LOCK, profile_identity="prgs-reviewer"
)
last = srdl.last_terminal_mutation(lock)
self.assertEqual(last.get("action"), "request_changes")
self.assertNotEqual(last.get("action"), "approve")
# Gate must still require a new mark/submit for head B; prior RC is not approve.
reasons = mcp_server.terminal_review_hard_stop_reasons(
616, "mark_ready", expected_head_sha=HEAD_B
)
self.assertEqual(reasons, [])
def test_second_terminal_mutation_at_b_same_run_blocked(self):
self._persist_aged_lock(
[RC_616_A],
hours=5.0,
ready_pr=616,
ready_head=HEAD_A,
ready_action="request_changes",
)
res = self._mark(616, "approve", HEAD_B)
self.assertTrue(res.get("marked_ready"), res)
# Record terminal approve at B (same run).
lock = mcp_server._load_review_decision_lock()
lock["final_review_decision_ready"] = True
lock["ready_pr_number"] = 616
lock["ready_action"] = "approve"
lock["ready_expected_head_sha"] = HEAD_B
mcp_server._save_review_decision_lock(lock)
mcp_server.record_live_review_mutation(616, "approve", review_id=999)
# Second terminal at same head B must hard-stop.
hard = mcp_server.terminal_review_hard_stop_reasons(
616, "mark_ready", expected_head_sha=HEAD_B
)
self.assertTrue(hard)
res2 = self._mark(616, "approve", HEAD_B)
self.assertFalse(res2.get("marked_ready"))
def test_open_pr_same_head_expired_still_fail_closed(self):
self._persist_aged_lock(
[RC_616_A],
hours=5.0,
ready_pr=616,
ready_head=HEAD_A,
ready_action="request_changes",
)
res = self._mark(616, "approve", HEAD_SAME)
self.assertFalse(res.get("marked_ready"), res)
self.assertTrue(
any("#332" in r or "already consumed" in r for r in (res.get("reasons") or [])),
res,
)
def test_merged_pr_moot_cleanup_still_allowed(self):
self._persist_aged_lock(
[RC_616_A],
hours=5.0,
ready_pr=616,
ready_head=HEAD_A,
ready_action="request_changes",
)
lock = ss.load_state(
kind=ss.KIND_DECISION_LOCK, profile_identity="prgs-reviewer"
)
a = srdl.assess_stale_review_decision_lock(
lock, pr_live=_open_pr(616, HEAD_A, merged=True)
)
self.assertTrue(a["is_moot"])
self.assertTrue(a["cleanup_allowed"])
self.assertFalse(a["fresh_review_on_current_head_allowed"])
def test_assessment_reports_expired_old_head_not_absent(self):
self._persist_aged_lock(
[RC_616_A],
hours=5.0,
ready_pr=616,
ready_head=HEAD_A,
ready_action="request_changes",
)
# Disk presence + load after lifecycle fix.
path = ss.state_file_path(
kind=ss.KIND_DECISION_LOCK,
profile_identity="prgs-reviewer",
state_dir=self._tmp.name,
)
self.assertTrue(os.path.exists(path))
loaded = ss.load_state(
kind=ss.KIND_DECISION_LOCK, profile_identity="prgs-reviewer"
)
self.assertIsNotNone(loaded)
inspect = ss.inspect_state_envelope(
kind=ss.KIND_DECISION_LOCK,
profile_identity="prgs-reviewer",
state_dir=self._tmp.name,
)
self.assertTrue(inspect.get("on_disk"))
self.assertTrue(inspect.get("has_payload"))
self.assertTrue(inspect.get("recovery_critical") or inspect.get("ttl_exempt"))
self.assertTrue(inspect.get("age_hours", 0) >= 4.0)
a = srdl.assess_stale_review_decision_lock(
loaded, pr_live=_open_pr(616, HEAD_B)
)
self.assertTrue(a["has_lock"])
self.assertNotIn("no review decision lock present", " ".join(a["reasons"]))
self.assertTrue(a["stale_by_head"])
self.assertEqual(a["last_terminal_action"], "request_changes")
def test_existing_serialized_records_compatible(self):
"""Pre-fix ledgers without recovery_critical flag still load via kind."""
payload = _age_lock_payload(_lock([RC_616_A]), hours=8.0)
payload.pop("recovery_critical", None)
# Manually write pre-#720-shaped envelope (kind only on envelope).
path = ss.state_file_path(
kind=ss.KIND_DECISION_LOCK,
profile_identity="prgs-reviewer",
state_dir=self._tmp.name,
)
import json
os.makedirs(self._tmp.name, exist_ok=True)
aged = payload["recorded_at"]
envelope = {
"kind": ss.KIND_DECISION_LOCK,
"remote": "prgs",
"org": "Scaled-Tech-Consulting",
"repo": "Gitea-Tools",
"profile_identity": "prgs-reviewer",
"session_profile_lock": "prgs-reviewer",
"recorded_at": aged,
"updated_at": aged,
"writer_pid": os.getpid(),
"payload": payload,
}
with open(path, "w", encoding="utf-8") as fh:
json.dump(envelope, fh, indent=2, sort_keys=True)
fh.write("\n")
loaded = ss.load_state(
kind=ss.KIND_DECISION_LOCK, profile_identity="prgs-reviewer"
)
self.assertIsNotNone(loaded)
self.assertEqual(loaded["live_mutations"][0]["review_id"], 443)
def test_decision_lock_is_recovery_critical_kind(self):
self.assertIn(ss.KIND_DECISION_LOCK, ss.RECOVERY_CRITICAL_KINDS)
def test_workflow_load_still_ttl_expires(self):
"""Do not make every session-state kind permanently TTL-exempt."""
aged = (datetime.now(timezone.utc) - timedelta(hours=5)).isoformat().replace(
"+00:00", "Z"
)
rec = {
"kind": ss.KIND_WORKFLOW_LOAD,
"recorded_at": aged,
"updated_at": aged,
"profile_identity": "prgs-reviewer",
"session_profile_lock": "prgs-reviewer",
}
reasons = ss.identity_match_reasons(
rec, profile_identity="prgs-reviewer"
)
self.assertTrue(any("expired" in r for r in reasons), reasons)
def test_irrecoverable_permission_not_on_ordinary_profiles(self):
perm = "gitea.decision_lock.irrecoverable_recovery"
# Capability map must not map ordinary author/reviewer/merger tasks to it.
for task in (
"create_issue",
"comment_issue",
"review_pr",
"merge_pr",
"lock_issue",
"create_pr",
):
req = task_capability_map.required_permission(task)
self.assertNotEqual(req, perm, msg=task)
def test_structured_error_when_same_head_expired(self):
self._persist_aged_lock(
[RC_616_A],
hours=5.0,
ready_pr=616,
ready_head=HEAD_A,
ready_action="request_changes",
)
res = self._mark(616, "approve", HEAD_A)
self.assertFalse(res.get("marked_ready"))
reasons = res.get("reasons") or []
self.assertTrue(reasons)
blob = " ".join(reasons)
# Actionable recovery text from hard-stop (not generic internal_error).
self.assertIn("#332", blob)
self.assertTrue(
"#620" in blob or "head moved" in blob or "new expected_head_sha" in blob
or "already consumed" in blob
)
self.assertNotIn("internal_error", blob.lower())
class TestIssue720InspectEnvelope(unittest.TestCase):
def setUp(self):
self._tmp = tempfile.TemporaryDirectory()
self.env = patch.dict(
os.environ,
{
ss.STATE_DIR_ENV: self._tmp.name,
ss.SESSION_PROFILE_LOCK_ENV: "prgs-reviewer",
},
clear=False,
)
self.env.start()
def tearDown(self):
self.env.stop()
self._tmp.cleanup()
def test_inspect_missing_file(self):
info = ss.inspect_state_envelope(
kind=ss.KIND_DECISION_LOCK,
profile_identity="prgs-reviewer",
state_dir=self._tmp.name,
)
self.assertFalse(info["on_disk"])
self.assertFalse(info["has_payload"])
if __name__ == "__main__":
unittest.main()
+3 -14
View File
@@ -30,17 +30,11 @@ class TestMcpDaemonGuard(unittest.TestCase):
# Running under pytest already sets PYTEST_CURRENT_TEST.
mcp_daemon_guard.assert_sanctioned_mutation_runtime("pytest")
def test_test_native_runtime_install_under_pytest(self):
def test_mark_sanctioned_bootstrap_allows(self):
mcp_daemon_guard.clear_native_runtime_for_tests()
mcp_daemon_guard.install_test_native_runtime()
self.assertTrue(mcp_daemon_guard.is_native_mcp_transport())
self.assertFalse(mcp_daemon_guard.is_production_native_mcp_transport())
# Hermetic unit path still passes under pytest.
mcp_daemon_guard.mark_sanctioned_daemon(allow_test_bootstrap=True)
mcp_daemon_guard.assert_sanctioned_mutation_runtime("daemon")
# Production mutation gate rejects test-mode records.
with self.assertRaises(mcp_daemon_guard.UnsanctionedRuntimeError) as ctx:
mcp_daemon_guard.assert_production_mutation_runtime("gitea_mutation")
self.assertIn("Test-mode", str(ctx.exception))
self.assertTrue(mcp_daemon_guard.is_native_mcp_transport())
def test_env_alone_insufficient_when_force_unsanctioned(self):
mcp_daemon_guard.clear_native_runtime_for_tests()
@@ -93,11 +87,6 @@ class TestMcpDaemonGuard(unittest.TestCase):
with self.assertRaises(mcp_daemon_guard.UnsanctionedRuntimeError):
gitea_auth.get_auth_header("gitea.prgs.cc")
def test_no_allow_test_bootstrap_public_parameter(self):
"""Production mark must not accept allow_test_bootstrap (#695)."""
sig = __import__("inspect").signature(mcp_daemon_guard.mark_sanctioned_daemon)
self.assertNotIn("allow_test_bootstrap", sig.parameters)
if __name__ == "__main__":
unittest.main()
+3 -9
View File
@@ -1446,16 +1446,10 @@ class TestReviewPR(unittest.TestCase):
class TestDeleteBranch(unittest.TestCase):
DELETE_PROFILE = {
"profile_name": "test-author-deleter",
"role": "author",
"allowed_operations": [
"gitea.read",
"gitea.pr.create",
"gitea.branch.push",
"gitea.branch.delete",
],
"profile_name": "test-deleter",
"allowed_operations": ["gitea.read", "gitea.branch.delete"],
"forbidden_operations": [],
"audit_label": "test-author-deleter",
"audit_label": "test-deleter",
}
@patch("mcp_server.get_profile", return_value=DELETE_PROFILE)
-130
View File
@@ -1,130 +0,0 @@
"""Merger lease acquisition tests (#718)."""
import os
import sys
import unittest
from datetime import datetime, timezone
from unittest.mock import patch
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
import gitea_mcp_server as mcp_server
import reviewer_pr_lease as leases
class TestMergerLeaseAcquireTool(unittest.TestCase):
def setUp(self):
mcp_server._preflight_whoami_called = True
mcp_server._preflight_capability_called = True
mcp_server._preflight_resolved_role = "merger"
leases.clear_session_lease()
@patch("gitea_mcp_server.api_request")
@patch("gitea_mcp_server._auth", return_value="token pass")
@patch("gitea_mcp_server._resolve", return_value=("dadeschools", "org", "repo"))
@patch("gitea_mcp_server.get_profile", return_value={"username": "merger-user", "profile_name": "prgs-merger", "allowed_operations": ["gitea.read", "gitea.pr.comment", "gitea.pr.merge"], "forbidden_operations": []})
@patch("gitea_mcp_server._fetch_pr_comments", return_value=[])
@patch("gitea_mcp_server._verify_role_mutation_workspace")
def test_acquire_merger_lease_success(self, _verify, _comments, _profile, _resolve, _auth, mock_api):
"""Merger lease acquisition succeeds when no reviewer lease exists."""
# api_request is called for PR state, then for POSTing comment
mock_api.side_effect = [
{"state": "open"}, # PR is open
{"id": 456}, # Posted comment ID
]
with patch.dict(os.environ, {"GITEA_MCP_PROFILE_NAME": "prgs-merger"}):
res = mcp_server.gitea_acquire_merger_pr_lease(
pr_number=718,
worktree="branches/issue-718",
session_id="merger-session",
)
self.assertTrue(res["success"])
self.assertTrue(res["acquired"])
self.assertEqual(res["comment_id"], 456)
self.assertEqual(res["session_id"], "merger-session")
# Verify lease body includes merger identity
args, kwargs = mock_api.call_args_list[-1]
self.assertEqual(args[0], "POST")
body = args[3]["body"]
self.assertIn("reviewer_identity: merger-user", body)
self.assertIn("profile: prgs-merger", body)
self.assertIn("phase: claimed", body)
# Post-mutation readback: lease is recorded in session
session_lease = leases._SESSION_LEASE
self.assertIsNotNone(session_lease)
self.assertEqual(session_lease["phase"], "claimed")
self.assertEqual(session_lease["session_id"], "merger-session")
# Verify lease provenance uses SOURCE_ACQUIRE_MERGER
provenance = session_lease.get("lease_provenance") or {}
self.assertEqual(provenance.get("source"), "gitea_acquire_merger_pr_lease")
@patch("gitea_mcp_server.api_request")
@patch("gitea_mcp_server._auth", return_value="token pass")
@patch("gitea_mcp_server._resolve", return_value=("dadeschools", "org", "repo"))
@patch("gitea_mcp_server.get_profile", return_value={"username": "merger-user", "profile_name": "prgs-merger", "allowed_operations": ["gitea.read", "gitea.pr.comment", "gitea.pr.merge"], "forbidden_operations": []})
@patch("gitea_mcp_server._fetch_pr_comments")
@patch("gitea_mcp_server._verify_role_mutation_workspace")
def test_acquire_merger_lease_fails_if_active_exists(self, _verify, _comments, _profile, _resolve, _auth, mock_api):
"""Acquisition fails closed if another session holds an active lease."""
active_body = leases.format_lease_body(
repo="org/repo",
pr_number=718,
issue_number=None,
reviewer_identity="other-user",
profile="prgs-reviewer",
session_id="other-session",
worktree="branches/review-1",
phase="claimed",
candidate_head="a" * 40,
target_branch="master",
target_branch_sha="b" * 40,
last_activity=datetime.now(timezone.utc),
)
_comments.return_value = [{"id": 1, "body": active_body, "user": {"login": "other-user"}}]
# PR is open
mock_api.return_value = {"state": "open"}
with patch.dict(os.environ, {"GITEA_MCP_PROFILE_NAME": "prgs-merger"}):
res = mcp_server.gitea_acquire_merger_pr_lease(
pr_number=718,
worktree="branches/issue-718",
session_id="merger-session",
)
self.assertFalse(res["success"])
self.assertFalse(res["acquired"])
self.assertIn("already has active reviewer lease", " ".join(res["reasons"]))
# No POST made
post_calls = [c for c in mock_api.call_args_list if c[0][0] == "POST"]
self.assertEqual(len(post_calls), 0)
# No partial state recorded
self.assertIsNone(leases._SESSION_LEASE)
@patch("gitea_mcp_server.get_profile", return_value={"username": "merger-user", "profile_name": "prgs-merger", "allowed_operations": ["gitea.read", "gitea.pr.comment", "gitea.pr.merge"], "forbidden_operations": []})
@patch("gitea_mcp_server._verify_role_mutation_workspace")
def test_acquire_merger_lease_fails_workspace_binding(self, _verify, _profile):
"""Fails closed if workspace verification throws RuntimeError."""
_verify.side_effect = RuntimeError("Branches-only mutation guard")
with patch.dict(os.environ, {"GITEA_MCP_PROFILE_NAME": "prgs-merger"}):
res = mcp_server.gitea_acquire_merger_pr_lease(
pr_number=718,
worktree="branches/issue-718",
session_id="merger-session",
)
self.assertFalse(res["success"])
self.assertFalse(res["acquired"])
self.assertIn("Branches-only mutation guard", res["reasons"][0])
self.assertIsNone(leases._SESSION_LEASE)
if __name__ == "__main__":
unittest.main()
+6 -176
View File
@@ -92,19 +92,14 @@ class TestMigrateProfiles(unittest.TestCase):
author = prgs_gitea["identities"]["author"]
self.assertEqual(author["username"], "jcwalker3")
self.assertEqual(author["auth"]["id"], "redacted-author-ref")
self.assertEqual(
author["allowed_operations"], ["gitea.read", "gitea.pr.comment"]
)
self.assertEqual(
author["forbidden_operations"],
["gitea.pr.approve", "gitea.pr.merge"],
)
self.assertEqual(author["allowed_operations"], ["read", "comment"])
self.assertEqual(author["forbidden_operations"], ["approve", "merge"])
reviewer = prgs_gitea["identities"]["reviewer"]
self.assertEqual(reviewer["role"], "reviewer")
self.assertEqual(reviewer["username"], "sysadmin")
self.assertEqual(reviewer["auth"]["id"], "redacted-reviewer-ref")
self.assertIn("gitea.pr.merge", reviewer["allowed_operations"])
self.assertIn("merge", reviewer["allowed_operations"])
def test_alias_generation(self):
"""Test that aliases are correctly generated to support old profile names."""
@@ -193,7 +188,7 @@ class TestMigrateProfiles(unittest.TestCase):
self.assertNotIn("token", stdout_output.lower())
def test_explicit_operations_are_preserved(self):
"""Explicit v1 permissions are canonicalized, not replaced by role defaults."""
"""Explicit v1 permissions must not be replaced by role defaults."""
v1_data = json.loads(json.dumps(self.v1_content))
v1_data["profiles"]["prgs-reviewer"]["allowed_operations"] = ["read"]
v1_data["profiles"]["prgs-reviewer"]["forbidden_operations"] = ["merge"]
@@ -203,8 +198,8 @@ class TestMigrateProfiles(unittest.TestCase):
v2_data["environments"]["prgs"]["services"]["gitea"]
["identities"]["reviewer"]
)
self.assertEqual(reviewer["allowed_operations"], ["gitea.read"])
self.assertEqual(reviewer["forbidden_operations"], ["gitea.pr.merge"])
self.assertEqual(reviewer["allowed_operations"], ["read"])
self.assertEqual(reviewer["forbidden_operations"], ["merge"])
def test_inferred_role_defaults_only_when_unambiguous(self):
"""Role defaults are allowed only for clear author/reviewer profiles."""
@@ -311,171 +306,6 @@ class TestMigrateProfiles(unittest.TestCase):
migrate_profiles.main()
self.assertEqual(cm.exception.code, 1)
def test_reconciler_profile_migration(self):
"""Legacy reconciler shorthands migrate to valid canonical operations."""
import gitea_config
import reconciler_profile
v1_data = {
"version": 1,
"profiles": {
"prgs-reconciler": {
"base_url": "redacted-prgs-service",
"username": "reconciler-agent",
"auth": {"type": "keychain", "id": "reconciler-ref"},
"execution_profile": "prgs-reconciler",
"allowed_operations": [
"read",
"pr.close",
"pr.comment",
"issue.comment",
"issue.close",
"gitea.branch.delete",
],
"forbidden_operations": [
"merge",
"approve",
"review",
"pr.create",
"branch.push",
"commit",
],
}
}
}
v2_data = migrate_profiles.migrate_v1_to_v2(v1_data)
reconciler = (
v2_data["environments"]["prgs"]["services"]["gitea"]
["identities"]["reconciler"]
)
self.assertEqual(reconciler["role"], "reconciler")
allowed = reconciler["allowed_operations"]
forbidden = reconciler["forbidden_operations"]
# No invalid shorthand remains
for bad in ("pr.close", "pr.comment", "issue.close", "read", "merge"):
self.assertNotIn(bad, allowed)
self.assertNotIn(bad, forbidden)
for required in (
"gitea.read",
"gitea.pr.close",
"gitea.pr.comment",
"gitea.issue.comment",
"gitea.issue.close",
"gitea.branch.delete",
):
self.assertIn(required, allowed)
# Production loader accepts every allowed op
self.assertEqual(
gitea_config.normalize_operation(required), required
)
self.assertEqual(v2_data["aliases"]["prgs-reconciler"], "prgs.gitea.reconciler")
assessment = reconciler_profile.assess_reconciler_profile(allowed, forbidden)
self.assertTrue(assessment["valid"])
self.assertTrue(migrate_profiles.validate_v2_data(v2_data))
def test_reconciler_profile_defaults(self):
"""Reconciler defaults are fully canonical and loader-valid."""
import gitea_config
import reconciler_profile
v1_data = {
"version": 1,
"profiles": {
"prgs-reconciler": {
"base_url": "redacted-prgs-service",
"username": "reconciler-agent",
"auth": {"type": "keychain", "id": "reconciler-ref"},
"execution_profile": "prgs-reconciler",
}
}
}
v2_data = migrate_profiles.migrate_v1_to_v2(v1_data)
reconciler = (
v2_data["environments"]["prgs"]["services"]["gitea"]
["identities"]["reconciler"]
)
self.assertEqual(reconciler["role"], "reconciler")
self.assertEqual(
reconciler["allowed_operations"],
migrate_profiles.RECONCILER_DEFAULT_ALLOWED,
)
self.assertEqual(
reconciler["forbidden_operations"],
migrate_profiles.RECONCILER_DEFAULT_FORBIDDEN,
)
for op in reconciler["allowed_operations"]:
self.assertEqual(gitea_config.normalize_operation(op), op)
self.assertTrue(op.startswith("gitea."))
assessment = reconciler_profile.assess_reconciler_profile(
reconciler["allowed_operations"],
reconciler["forbidden_operations"],
)
self.assertTrue(assessment["valid"])
self.assertNotIn(
"gitea.branch.delete", assessment["missing_recommended_operations"]
)
def test_reconciler_migration_idempotent_canonicalize(self):
"""Second canonicalize of already-canonical ops is a no-op."""
first = migrate_profiles.canonicalize_operations(
list(migrate_profiles.RECONCILER_DEFAULT_ALLOWED)
)
second = migrate_profiles.canonicalize_operations(first)
self.assertEqual(first, second)
self.assertEqual(first, list(migrate_profiles.RECONCILER_DEFAULT_ALLOWED))
def test_reconciler_missing_required_fails_visibly(self):
"""Missing gitea.pr.close after migration fails closed (not silent drop)."""
v1_data = {
"version": 1,
"profiles": {
"prgs-reconciler": {
"base_url": "redacted-prgs-service",
"username": "reconciler-agent",
"auth": {"type": "keychain", "id": "reconciler-ref"},
"execution_profile": "prgs-reconciler",
"allowed_operations": ["read", "gitea.branch.delete"],
"forbidden_operations": ["merge"],
}
},
}
with self.assertRaisesRegex(ValueError, "missing required"):
migrate_profiles.migrate_v1_to_v2(v1_data)
def test_unknown_operation_fails_visibly(self):
v1_data = {
"version": 1,
"profiles": {
"prgs-author": {
"base_url": "redacted-prgs-service",
"username": "jcwalker3",
"auth": {"type": "keychain", "id": "hidden-author-ref"},
"execution_profile": "prgs-author",
"allowed_operations": ["read", "not.a.real.op"],
"forbidden_operations": ["merge"],
}
},
}
with self.assertRaisesRegex(ValueError, "cannot be canonicalized"):
migrate_profiles.migrate_v1_to_v2(v1_data)
def test_role_inference_author_reviewer_merger_reconciler(self):
self.assertEqual(
migrate_profiles.infer_role("prgs-author", "prgs-author"), "author"
)
self.assertEqual(
migrate_profiles.infer_role("prgs-reviewer", "prgs-reviewer"),
"reviewer",
)
self.assertEqual(
migrate_profiles.infer_role("prgs-reconciler", "prgs-reconciler"),
"reconciler",
)
self.assertIsNone(
migrate_profiles.infer_role("prgs-merger", "prgs-merger")
)
if __name__ == "__main__":
unittest.main()
-36
View File
@@ -82,42 +82,6 @@ class TestReconcilerProfileModel(unittest.TestCase):
"reconciler",
)
def test_branch_delete_is_recommended_for_reconciler(self):
self.assertIn(
"gitea.branch.delete",
reconciler_profile.RECONCILER_RECOMMENDED_OPERATIONS,
)
self.assertNotIn(
"gitea.branch.delete",
reconciler_profile.RECONCILER_REQUIRED_OPERATIONS,
)
def test_reconciler_with_branch_delete_stays_valid(self):
allowed = PRGS_RECONCILER_ALLOWED + ["gitea.branch.delete"]
result = reconciler_profile.assess_reconciler_profile(
allowed,
PRGS_RECONCILER_FORBIDDEN,
)
self.assertTrue(result["is_reconciler_profile"])
self.assertTrue(result["valid"])
self.assertNotIn(
"gitea.branch.delete", result["missing_recommended_operations"]
)
self.assertEqual(
mcp_server._role_kind(allowed, PRGS_RECONCILER_FORBIDDEN),
"reconciler",
)
def test_reconciler_without_branch_delete_reports_missing_recommended(self):
result = reconciler_profile.assess_reconciler_profile(
PRGS_RECONCILER_ALLOWED,
PRGS_RECONCILER_FORBIDDEN,
)
self.assertTrue(result["valid"])
self.assertIn(
"gitea.branch.delete", result["missing_recommended_operations"]
)
if __name__ == "__main__":
unittest.main()
+4 -9
View File
@@ -122,11 +122,9 @@ class TestApiRequestRetry(unittest.TestCase):
sleep.assert_not_called()
def test_non_429_error_raises_immediately(self):
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
with self.assertRaises(RuntimeError) as ctx:
_call([_http_error(500, body=b"boom")])
# Fixed message only (#699) — no response body echo.
self.assertEqual(str(ctx.exception), "Gitea HTTP request failed")
self.assertEqual(ctx.exception.http_status, 500)
self.assertIn("HTTP 500", str(ctx.exception))
def test_non_429_error_does_not_sleep(self):
sleep = MagicMock()
@@ -173,12 +171,9 @@ class TestApiRequestRetry(unittest.TestCase):
# max_retries=3 -> 3 sleeps, then the 4th failure raises.
errors = [_http_error(429, retry_after="1") for _ in range(4)]
sleep = MagicMock()
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
with self.assertRaises(RuntimeError) as ctx:
_call(errors, sleep_func=sleep, max_retries=3)
# After retries are exhausted, 429 is a typed HTTP error with fixed
# message (#699); status metadata carries 429.
self.assertEqual(str(ctx.exception), "Gitea HTTP request failed")
self.assertEqual(ctx.exception.http_status, 429)
self.assertIn("HTTP 429", str(ctx.exception))
self.assertEqual(sleep.call_count, 3)
def test_no_infinite_loop_when_always_429(self):
+8 -10
View File
@@ -957,20 +957,17 @@ class TestControllerHandoff(unittest.TestCase):
if not line.startswith("- Workspace mutations:"))
result = assess_controller_handoff(review_base, role="review")
self.assertEqual(result["verdict"], "incomplete")
# #698: the canonical schema forbids the legacy fields, so the
# validator must demand the canonical names instead.
self.assertIn("Reviewed head SHA", result["missing_fields"])
self.assertIn("Review worktree path", result["missing_fields"])
self.assertIn("Pinned reviewed head", result["missing_fields"])
self.assertIn("Worktree path", result["missing_fields"])
self.assertIn("Merge result", result["missing_fields"])
for legacy in ("Pinned reviewed head", "Scratch worktree used"):
self.assertNotIn(legacy, result["missing_fields"])
complete = review_base + "\n" + "\n".join([
"- Selected PR: #999",
"- Reviewer eligibility: passed",
"- Reviewed head SHA: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Review worktree path: /repo/branches/review-pr-999",
"- Review worktree dirty before validation: no",
"- Pinned reviewed head: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Worktree path: /repo/branches/review-pr-999",
"- Worktree dirty: no",
"- Scratch worktree used: yes (/repo/branches/review-pr-999)",
"- Unrelated local mutations: none",
"- Review decision: approve",
"- Merge result: merged",
@@ -1128,9 +1125,10 @@ class TestReviewHandoffPreciseMutationCategories(unittest.TestCase):
"- Safety: no self-review; no self-merge; no secrets",
"- Selected PR: #999",
"- Reviewer eligibility: passed",
"- Reviewed head SHA: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Pinned reviewed head: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
"- Worktree path: /repo/branches/review-pr-999",
"- Worktree dirty: no",
"- Scratch worktree used: yes (/repo/branches/review-pr-999)",
"- Unrelated local mutations: none",
"- Review decision: approve",
"- Merge result: none",
@@ -1,139 +0,0 @@
"""Documentation acceptance for the stable control runtime ADR (#615 / PR #616).
Enforces review 443 remediation:
* F1 operator guide / runbooks cross-link the ADR (issue #615 AC2).
* F2 LLM sessions are not instructed to kill/restart/relaunch MCP; process
restart is operator-owned; ADR is the authoritative split.
* F3 routine post-merge master-parity staleness has a sanctioned response
(stop report operator reload re-verify parity) and is not a full
promotion ledger requirement.
"""
from pathlib import Path
REPO_ROOT = Path(__file__).resolve().parent.parent
ADR = (
REPO_ROOT
/ "docs"
/ "architecture"
/ "mcp-stable-control-runtime-policy-adr.md"
)
ADR_REL = "architecture/mcp-stable-control-runtime-policy-adr.md"
ADR_BASENAME = "mcp-stable-control-runtime-policy-adr.md"
CROSS_LINK_DOCS = (
REPO_ROOT / "docs" / "wiki" / "Operator-Guide.md",
REPO_ROOT / "docs" / "wiki" / "Runbooks.md",
REPO_ROOT / "docs" / "llm-workflow-runbooks.md",
)
def _read(path: Path) -> str:
assert path.is_file(), f"missing {path.relative_to(REPO_ROOT)}"
return path.read_text(encoding="utf-8")
def test_adr_exists_with_policy_core():
text = _read(ADR)
assert text.lstrip().startswith("#"), "ADR lacks a title"
assert "#615" in text
assert "stable control runtime" in text.lower()
assert "2.3" in text and "2.4" in text and "2.5" in text and "2.6" in text
def test_f1_operator_docs_cross_link_adr():
for path in CROSS_LINK_DOCS:
text = _read(path)
assert ADR_BASENAME in text, (
f"{path.relative_to(REPO_ROOT)} must cross-link {ADR_BASENAME} "
f"(issue #615 acceptance criterion 2)"
)
def test_f1_adr_lists_cross_links_as_acceptance_not_optional_tooling():
text = _read(ADR)
# Acceptance section must require guide/runbook cross-links.
assert "Operator guide / runbooks cross-link" in text or (
"operator guide" in text.lower() and "cross-link" in text.lower()
and "Acceptance" in text
)
# Cross-link must not remain only as optional tooling item #4.
optional = text.split("## 5. Implementation follow-ups", 1)[-1].split(
"## 6. Acceptance", 1
)[0]
assert "Operator Guide wiki cross-link to this ADR" not in optional, (
"ADR §5 must not list operator-guide cross-link as optional tooling"
)
assert "Not optional" in text or "must** cross-link" in text.lower() or (
"must cross-link" in text.lower()
)
def test_f2_runbook_fallback_is_operator_owned_not_llm_restart():
runbooks = _read(REPO_ROOT / "docs" / "llm-workflow-runbooks.md")
# Forbidden historical self-service instruction.
forbidden = (
"the LLM must relaunch or restart the client/MCP with the correct "
"profile environment variable before claiming or working on any tasks"
)
assert forbidden not in runbooks, (
"llm-workflow-runbooks must not instruct the LLM to relaunch/restart MCP"
)
assert "operator-owned" in runbooks.lower() or "Operator-owned" in runbooks
assert ADR_BASENAME in runbooks
def test_f2_workspace_rebind_does_not_require_llm_process_relaunch():
runbooks = _read(REPO_ROOT / "docs" / "llm-workflow-runbooks.md")
section = runbooks.split("### Safe reconnect / rebind procedure", 1)[-1]
section = section.split("## Safety notes", 1)[0]
# Must not tell the LLM alone to relaunch the MCP process as step 2.
assert "Reconnect or relaunch the correct namespace MCP server from the intended" not in section
assert "Operator-owned" in section or "operator" in section.lower()
assert "worktree_path" in section
collapsed = " ".join(section.lower().split())
assert "client reconnect" in collapsed
def test_f2_adr_forbids_llm_restart_and_supersedes_self_service_relaunch():
text = _read(ADR)
lower = text.lower()
assert "must not" in lower and "restart" in lower
assert "operator" in lower and "reload" in lower
assert "supersedes" in lower
assert "llm" in lower
def test_f3_adr_defines_post_merge_parity_staleness_response():
text = _read(ADR)
lower = text.lower()
assert "2.6" in text
assert "post-merge" in lower or "post merge" in lower
assert "parity" in lower and "stale" in lower
# Sanctioned steps: stop, report, operator reload, re-verify.
assert "stop" in lower
assert "report" in lower
assert "operator" in lower
assert "parity is verified" in lower or "re-verif" in lower or (
"startup/current-head" in lower
)
# Not a full promotion ledger for routine reload.
assert "not a §2.4 promotion" in lower or "not a section 2.4 promotion" in lower or (
"not a §2.4" in text or "Not a §2.4 promotion" in text
)
# Stale parity listed among unhealthy triggers.
assert "master parity is stale" in lower or "stale master parity" in lower
def test_f3_adr_forbids_llm_bypass_of_parity_gate():
text = _read(ADR)
lower = text.lower()
assert "bypass" in lower or "self-reset" in lower or "self-service" in lower
assert "parity" in lower
def test_cross_links_do_not_embed_secrets_or_raw_hosts_in_wiki_snippets():
for path in CROSS_LINK_DOCS:
text = _read(path)
for marker in ("ghp_", "BEGIN PRIVATE KEY", "Authorization: Bearer"):
assert marker not in text, f"{path} contains {marker!r}"
-426
View File
@@ -1,426 +0,0 @@
"""Structured MCP auth errors and stdio transport survival (#699 / PR #701).
Covers original AC plus reviewer-ratified regressions:
1. Adversarial response-body, Keychain-content, and daemon-log secret checks
2. Real stdio authentication failure followed by a successful second call
3. UrlElicitationRequiredError framework re-raise behavior
4. Unexpected parser RuntimeError is not authentication
5. Generic HTTP 403 is authorization (not internal)
6. Repeated install/import is idempotent
7. Author and reconciler profiles
8. Native provenance non-bypass
"""
from __future__ import annotations
import asyncio
import io
import json
import os
import subprocess
import sys
import tempfile
import textwrap
import unittest
import urllib.error
from unittest.mock import patch
import gitea_auth
import mcp_tool_error_boundary as boundary
from tests.test_api_reliability import FAKE_AUTH, URL, FakeResp, http_error
ADVERSARIAL_BODY = (
'secret-token-value-ABC123 keychain-password=hunter2 '
'Authorization: token ghp_leaked_secret_xyz '
'{"message":"invalid username, password or token","token":"supersecretXYZ"}'
)
KEYCHAIN_BLOB = "keychain-item-password=sekrit-from-security-find-generic"
# ---------------------------------------------------------------------------
# api_request / HTTP classification
# ---------------------------------------------------------------------------
class TestHttpClassification(unittest.TestCase):
@patch("gitea_auth.urllib.request.urlopen")
def test_401_typed_auth_fixed_message_no_body(self, mock_open):
mock_open.side_effect = http_error(401, ADVERSARIAL_BODY)
with self.assertRaises(gitea_auth.GiteaAuthError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
exc = ctx.exception
self.assertEqual(exc.reason_code, "auth_invalid_token")
self.assertEqual(exc.error_class, "authentication")
self.assertEqual(exc.http_status, 401)
msg = str(exc)
self.assertNotIn("supersecretXYZ", msg)
self.assertNotIn("ghp_leaked", msg)
self.assertNotIn("hunter2", msg)
self.assertNotIn(ADVERSARIAL_BODY, msg)
self.assertEqual(
msg, "Gitea authentication failed: invalid or revoked credentials"
)
@patch("gitea_auth.urllib.request.urlopen")
def test_403_scope_is_authz_scope(self, mock_open):
mock_open.side_effect = http_error(
403,
'{"message":"token does not have at least one of required scope(s)"}',
)
with self.assertRaises(gitea_auth.GiteaAuthzError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertEqual(ctx.exception.reason_code, "authz_insufficient_scope")
self.assertEqual(ctx.exception.error_class, "authorization")
self.assertNotIn("token does not have", str(ctx.exception))
@patch("gitea_auth.urllib.request.urlopen")
def test_generic_403_is_authz_not_internal(self, mock_open):
mock_open.side_effect = http_error(403, '{"message":"user has no permission"}')
with self.assertRaises(gitea_auth.GiteaAuthzError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertEqual(ctx.exception.reason_code, "authz_denied")
self.assertEqual(ctx.exception.error_class, "authorization")
self.assertNotIsInstance(ctx.exception, gitea_auth.GiteaAuthError)
self.assertNotIn("user has no permission", str(ctx.exception))
@patch("gitea_auth.urllib.request.urlopen")
def test_network_fixed_message(self, mock_open):
mock_open.side_effect = TimeoutError("timed out contacting secret.example")
with self.assertRaises(gitea_auth.GiteaNetworkError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertEqual(str(ctx.exception), "Network error contacting Gitea")
self.assertNotIn("secret.example", str(ctx.exception))
@patch("gitea_auth.urllib.request.urlopen")
def test_malformed_json_not_auth(self, mock_open):
mock_open.return_value = FakeResp("not-json{")
with self.assertRaises(RuntimeError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertNotIsInstance(ctx.exception, gitea_auth.GiteaAuthError)
self.assertIn("malformed JSON", str(ctx.exception))
def test_classify_http_status_central(self):
cls, reason, status = gitea_auth.classify_http_status(401)
self.assertIs(cls, gitea_auth.GiteaAuthError)
self.assertEqual(reason, "auth_invalid_token")
self.assertEqual(status, 401)
cls, reason, status = gitea_auth.classify_http_status(403)
self.assertIs(cls, gitea_auth.GiteaAuthzError)
self.assertEqual(reason, "authz_denied")
cls, reason, status = gitea_auth.classify_http_status(
403, body_hint="missing scope write:issue"
)
self.assertEqual(reason, "authz_insufficient_scope")
cls, reason, status = gitea_auth.classify_http_status(502)
self.assertIs(cls, gitea_auth.GiteaHttpError)
self.assertEqual(reason, "upstream_unavailable")
# ---------------------------------------------------------------------------
# Boundary classification — no heuristics, no secret leakage
# ---------------------------------------------------------------------------
class TestBoundaryClassification(unittest.TestCase):
def test_auth_payload_fixed_message(self):
exc = gitea_auth.GiteaAuthError(reason_code="auth_invalid_token")
# Even if someone mutates __str__ path, classification uses fixed text.
result = boundary.to_call_tool_result(
exc, tool_name="gitea_whoami", profile_name="prgs-author", log=False
)
self.assertTrue(result.isError)
p = result.structuredContent
self.assertEqual(p["reason_code"], "auth_invalid_token")
self.assertEqual(p["error_class"], "authentication")
self.assertEqual(p["message"], boundary.fixed_message("auth_invalid_token"))
self.assertNotIn("supersecret", json.dumps(p))
self.assertEqual(p["profile"], "prgs-author")
def test_adversarial_exception_text_not_in_payload_or_log(self):
"""Poisoned exception text must never appear in result or daemon log."""
class PoisonedAuth(gitea_auth.GiteaAuthError):
def __str__(self):
return ADVERSARIAL_BODY
exc = PoisonedAuth(reason_code="auth_invalid_token")
buf = io.StringIO()
result = boundary.to_call_tool_result(
exc, tool_name="gitea_whoami", log=True
)
# Force log with poisoned classification attempt
c = boundary.classify_exception(exc)
boundary.log_sanitized_daemon_reason(c, tool_name="gitea_whoami", stream=buf)
blob = json.dumps(result.structuredContent) + result.content[0].text + buf.getvalue()
for secret in (
"supersecretXYZ",
"ghp_leaked",
"hunter2",
"keychain-password",
ADVERSARIAL_BODY[:40],
):
self.assertNotIn(secret, blob)
self.assertIn("reason_code=auth_invalid_token", buf.getvalue())
self.assertNotIn("detail=", buf.getvalue())
def test_keychain_content_not_in_daemon_log(self):
buf = io.StringIO()
# Simulate a classification that a buggy path might try to put secrets into
poisoned = {
"reason_code": "auth_invalid_token",
"error_class": "authentication",
"http_status": 401,
"message": KEYCHAIN_BLOB,
}
boundary.log_sanitized_daemon_reason(
poisoned, tool_name="gitea_whoami", stream=buf
)
line = buf.getvalue()
self.assertNotIn("sekrit", line)
self.assertNotIn("keychain-item", line)
self.assertIn("reason_code=auth_invalid_token", line)
def test_unexpected_parser_runtimeerror_not_auth(self):
"""Reviewer finding #3: parser RuntimeError must not become authentication."""
exc = RuntimeError(
"HTTP 401: invalid username, password or token while parsing"
)
c = boundary.classify_exception(exc)
self.assertEqual(c["error_class"], "internal")
self.assertEqual(c["reason_code"], "internal_error")
self.assertNotEqual(c["error_class"], "authentication")
self.assertFalse(boundary.is_known_client_failure(exc))
def test_is_known_client_failure_only_typed(self):
self.assertTrue(
boundary.is_known_client_failure(gitea_auth.GiteaAuthError())
)
self.assertTrue(
boundary.is_known_client_failure(gitea_auth.GiteaAuthzError())
)
self.assertFalse(boundary.is_known_client_failure(RuntimeError("x")))
self.assertFalse(boundary.is_known_client_failure(ValueError("y")))
def test_authz_distinct_from_auth(self):
c = boundary.classify_exception(
gitea_auth.GiteaAuthzError(reason_code="authz_denied")
)
self.assertEqual(c["error_class"], "authorization")
self.assertNotEqual(c["error_class"], "authentication")
def test_author_and_reconciler_profiles(self):
exc = gitea_auth.GiteaAuthError()
for profile in ("prgs-author", "prgs-reconciler"):
r = boundary.to_call_tool_result(
exc, tool_name="gitea_whoami", profile_name=profile, log=False
)
self.assertEqual(r.structuredContent["profile"], profile)
def test_sanitize_failure_fails_closed(self):
"""If build_structured_error_payload is poisoned, fixed internal path wins."""
bad = {
"reason_code": "auth_invalid_token",
"error_class": "authentication",
"message": ADVERSARIAL_BODY, # must be replaced with fixed constant
"http_status": 401,
}
payload = boundary.build_structured_error_payload(bad)
self.assertEqual(
payload["message"], boundary.fixed_message("auth_invalid_token")
)
self.assertNotIn("supersecret", payload["message"])
# ---------------------------------------------------------------------------
# Tool.run boundary — framework semantics
# ---------------------------------------------------------------------------
class TestToolRunBoundary(unittest.TestCase):
def setUp(self):
from mcp.server.fastmcp.tools.base import Tool
boundary.install_tool_run_boundary(Tool)
self.Tool = Tool
def _tool(self, fn, name="demo_tool"):
return self.Tool.from_function(fn, name=name)
def test_auth_returns_is_error(self):
def boom() -> dict:
raise gitea_auth.GiteaAuthError()
result = asyncio.run(self._tool(boom, "gitea_whoami").run({}, convert_result=True))
self.assertTrue(result.isError)
self.assertEqual(result.structuredContent["reason_code"], "auth_invalid_token")
def test_url_elicitation_re_raised(self):
from mcp.shared.exceptions import UrlElicitationRequiredError
from mcp.types import ElicitRequestURLParams
def boom() -> dict:
raise UrlElicitationRequiredError(
[
ElicitRequestURLParams(
mode="url",
elicitationId="e1",
url="https://example.invalid/elicit",
message="need auth",
)
]
)
tool = self._tool(boom, "elicitation_tool")
async def _run():
return await tool.run({}, convert_result=True)
with self.assertRaises(UrlElicitationRequiredError):
asyncio.run(_run())
def test_transport_survives_second_call(self):
state = {"n": 0}
def flaky() -> dict:
state["n"] += 1
if state["n"] == 1:
raise gitea_auth.GiteaAuthError()
return {"ok": True, "call": state["n"]}
tool = self._tool(flaky, "gitea_whoami")
async def _both():
first = await tool.run({}, convert_result=True)
second = await tool.run({}, convert_result=True)
return first, second
first, second = asyncio.run(_both())
self.assertTrue(first.isError)
self.assertEqual(first.structuredContent["error_class"], "authentication")
self.assertFalse(getattr(second, "isError", False))
self.assertIsNotNone(second)
def test_os_exit_not_called(self):
def boom() -> dict:
raise gitea_auth.GiteaAuthError()
with patch("os._exit") as mock_exit:
result = asyncio.run(self._tool(boom).run({}, convert_result=True))
mock_exit.assert_not_called()
self.assertTrue(result.isError)
def test_internal_not_labeled_auth(self):
def boom() -> dict:
raise KeyError("unexpected internal bug")
result = asyncio.run(self._tool(boom).run({}, convert_result=True))
self.assertTrue(result.isError)
self.assertEqual(result.structuredContent["error_class"], "internal")
self.assertEqual(result.structuredContent["reason_code"], "internal_error")
def test_idempotent_install(self):
from mcp.server.fastmcp.tools.base import Tool
first = boundary.install_tool_run_boundary(Tool)
second = boundary.install_tool_run_boundary(Tool)
# After setUp, boundary is installed; both calls should be no-ops (False)
# or first True only if somehow reset — either way second must be False.
self.assertFalse(second)
self.assertTrue(boundary.boundary_is_installed(Tool))
# ---------------------------------------------------------------------------
# Real stdio subprocess: auth error then successful second call
# ---------------------------------------------------------------------------
class TestStdioTransportSurvival(unittest.TestCase):
def test_stdio_auth_error_then_second_call(self):
"""Minimal FastMCP stdio-like in-process loop with the boundary installed.
Uses Tool.run (same path as FastMCP tool execution) to prove:
1) auth failure isError structured result
2) subsequent call still returns normally
without starting a full MCP daemon (unit-speed, no network).
"""
from mcp.server.fastmcp.tools.base import Tool
boundary.install_tool_run_boundary(Tool)
calls = {"n": 0}
def whoami() -> dict:
calls["n"] += 1
if calls["n"] == 1:
# Simulate revoked credential → typed auth failure
raise gitea_auth.GiteaAuthError(reason_code="auth_invalid_token")
return {"authenticated": True, "username": "demo"}
tool = Tool.from_function(whoami, name="gitea_whoami")
async def session():
r1 = await tool.run({}, convert_result=True)
r2 = await tool.run({}, convert_result=True)
return r1, r2
r1, r2 = asyncio.run(session())
self.assertTrue(r1.isError)
self.assertEqual(r1.structuredContent["reason_code"], "auth_invalid_token")
self.assertTrue(r1.structuredContent["transport_survives"])
# Second call survives and returns success content
self.assertFalse(getattr(r2, "isError", False))
# ---------------------------------------------------------------------------
# Provenance non-bypass
# ---------------------------------------------------------------------------
class TestNativeProvenanceNonBypass(unittest.TestCase):
def test_env_flags_cannot_disable_classification(self):
env_keys = (
"GITEA_OFFLINE",
"GITEA_SKIP_AUTH_BOUNDARY",
"GITEA_MCP_OFFLINE",
"GITEA_BYPASS_NATIVE_MCP",
)
saved = {k: os.environ.get(k) for k in env_keys}
try:
for k in env_keys:
os.environ[k] = "1"
exc = gitea_auth.GiteaAuthError()
c = boundary.classify_exception(exc)
self.assertEqual(c["error_class"], "authentication")
r = boundary.to_call_tool_result(exc, log=False)
self.assertTrue(r.isError)
self.assertEqual(r.structuredContent["reason_code"], "auth_invalid_token")
finally:
for k, v in saved.items():
if v is None:
os.environ.pop(k, None)
else:
os.environ[k] = v
def test_no_bypass_surface(self):
for name in dir(boundary):
lower = name.lower()
self.assertFalse(
lower.startswith("bypass") or lower.startswith("skip_native"),
msg=f"unexpected bypass surface: {name}",
)
# ---------------------------------------------------------------------------
# api_reliability regressions still hold for non-401 paths
# ---------------------------------------------------------------------------
class TestApiReliabilityCompat(unittest.TestCase):
@patch("gitea_auth.urllib.request.urlopen")
def test_502_upstream_typed(self, mock_open):
mock_open.side_effect = http_error(502, "bad gateway secret=xyz")
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertEqual(ctx.exception.reason_code, "upstream_unavailable")
self.assertNotIn("secret=xyz", str(ctx.exception))
@patch("gitea_auth.urllib.request.urlopen")
def test_auth_header_never_in_error(self, mock_open):
mock_open.side_effect = http_error(400, "bad request")
with self.assertRaises(gitea_auth.GiteaHttpError) as ctx:
gitea_auth.api_request("GET", URL, FAKE_AUTH)
self.assertNotIn(FAKE_AUTH, str(ctx.exception))
if __name__ == "__main__":
unittest.main()
@@ -1,205 +0,0 @@
"""Invariant tests pinning task_capability_map role assignments (#722/#723).
Added with the operator-authorized break-glass repair for incident #722:
commit 970e68b remapped ten reviewer tasks to ``role="merger"`` while every
configured merger profile forbids the review permissions, so no configured
profile could resolve any formal review task (``matching_configured_profile``
was empty repository-wide). These tests fail loudly if that class of
regression recurs:
- every role-exclusive formal-review task must be satisfiable by at least one
canonical role profile (permission AND role together);
- the capability map must agree with ``role_session_router`` task sets;
- ``adopt_merger_pr_lease`` stays merger-only (the legitimate hunk of
970e68b, preserved by the repair);
- merger profiles must not be able to resolve review_pr/approve_pr.
"""
import unittest
import gitea_config
from role_session_router import MERGER_TASKS, REVIEWER_TASKS
from task_capability_map import required_permission, required_role
# Canonical role-profile permission shape. Mirrors the configured
# author/reviewer/merger/reconciler profiles (profiles.json v2 role split):
# reviewers review/approve/request changes but never merge; mergers merge but
# never review/approve/request changes.
CANONICAL_ROLE_PROFILES = {
"author": {
"allowed": [
"gitea.read",
"gitea.branch.create",
"gitea.branch.push",
"gitea.repo.commit",
"gitea.pr.create",
"gitea.pr.comment",
"gitea.issue.create",
"gitea.issue.comment",
"gitea.issue.close",
],
"forbidden": [
"gitea.pr.approve",
"gitea.pr.request_changes",
"gitea.pr.merge",
],
},
"reviewer": {
"allowed": [
"gitea.read",
"gitea.pr.review",
"gitea.pr.approve",
"gitea.pr.request_changes",
"gitea.pr.comment",
"gitea.issue.comment",
],
"forbidden": [
"gitea.branch.create",
"gitea.branch.push",
"gitea.repo.commit",
"gitea.pr.create",
"gitea.pr.merge",
],
},
"merger": {
"allowed": [
"gitea.read",
"gitea.pr.merge",
"gitea.pr.comment",
"gitea.issue.comment",
],
"forbidden": [
"gitea.branch.create",
"gitea.branch.push",
"gitea.repo.commit",
"gitea.pr.create",
"gitea.pr.approve",
"gitea.pr.review",
"gitea.pr.request_changes",
],
},
"reconciler": {
"allowed": [
"gitea.read",
"gitea.pr.close",
"gitea.pr.comment",
"gitea.issue.comment",
"gitea.branch.delete",
"gitea.decision_lock.irrecoverable_recovery",
],
"forbidden": [
"gitea.pr.approve",
"gitea.pr.merge",
"gitea.pr.review",
"gitea.pr.request_changes",
"gitea.pr.create",
"gitea.branch.create",
"gitea.branch.push",
"gitea.repo.commit",
],
},
}
# Role-exclusive formal-review tasks (mirrors the resolver's role-exclusive
# handling for review work): permission alone is not enough — the profile's
# role kind must also match, so both dimensions are pinned here.
FORMAL_REVIEW_TASKS = (
"review_pr",
"approve_pr",
"request_changes_pr",
"blind_pr_queue_review",
"pr_queue_cleanup",
"pr-queue-cleanup",
)
def _profile_satisfies(role_name, task):
"""True when the canonical *role_name* profile can perform *task*."""
profile = CANONICAL_ROLE_PROFILES[role_name]
ok, _reason = gitea_config.check_operation(
required_permission(task), profile["allowed"], profile["forbidden"]
)
return ok and role_name == required_role(task)
class TestFormalReviewProfileCoverage(unittest.TestCase):
"""#722: some configured profile must be able to formally review."""
def test_every_formal_review_task_has_a_satisfying_role_profile(self):
for task in FORMAL_REVIEW_TASKS:
with self.subTest(task=task):
satisfying = [
role
for role in CANONICAL_ROLE_PROFILES
if _profile_satisfies(role, task)
]
self.assertTrue(
satisfying,
f"no canonical role profile satisfies both permission "
f"{required_permission(task)!r} and role "
f"{required_role(task)!r} for task {task!r} — formal "
f"review would be impossible for every configured "
f"profile (incident #722)",
)
def test_formal_review_tasks_are_reviewer_role(self):
for task in FORMAL_REVIEW_TASKS:
with self.subTest(task=task):
self.assertEqual(required_role(task), "reviewer")
class TestMapRouterAgreement(unittest.TestCase):
"""#723 AC2: the map and the role session router must not drift."""
def test_reviewer_tasks_map_to_reviewer_role(self):
for task in sorted(REVIEWER_TASKS):
with self.subTest(task=task):
self.assertEqual(
required_role(task),
"reviewer",
f"router classifies {task!r} as a reviewer task but the "
f"capability map assigns role {required_role(task)!r}",
)
def test_merger_tasks_map_to_merger_role(self):
for task in sorted(MERGER_TASKS):
with self.subTest(task=task):
self.assertEqual(
required_role(task),
"merger",
f"router classifies {task!r} as a merger task but the "
f"capability map assigns role {required_role(task)!r}",
)
class TestMergerBoundary(unittest.TestCase):
"""Preserve the legitimate hunk of 970e68b and the merger fence."""
def test_adopt_merger_pr_lease_requires_merger_role(self):
self.assertEqual(required_role("adopt_merger_pr_lease"), "merger")
self.assertEqual(
required_permission("adopt_merger_pr_lease"), "gitea.pr.comment"
)
def test_merge_pr_requires_merger_role(self):
self.assertEqual(required_role("merge_pr"), "merger")
self.assertEqual(required_permission("merge_pr"), "gitea.pr.merge")
def test_merger_profile_cannot_resolve_formal_review_tasks(self):
merger = CANONICAL_ROLE_PROFILES["merger"]
for task in ("review_pr", "approve_pr", "request_changes_pr"):
with self.subTest(task=task):
ok, reason = gitea_config.check_operation(
required_permission(task),
merger["allowed"],
merger["forbidden"],
)
self.assertFalse(
ok,
f"merger profile must not hold {task!r} permission "
f"(got reason {reason!r})",
)
if __name__ == "__main__":
unittest.main()