diff --git a/docs/mcp-restart-classes.md b/docs/mcp-restart-classes.md new file mode 100644 index 0000000..5c93695 --- /dev/null +++ b/docs/mcp-restart-classes.md @@ -0,0 +1,53 @@ +# MCP restart classes and blast-radius permissions (#663) + +This is the machine-enforced class matrix used by +`restart_coordinator.RESTART_CLASS_POLICIES`. It implements the narrower-first +recovery ladder from #655 and the authorization policy from #656, using the +path inventory from #657 and the impact coordinator from #658. Product and +delivery lineage: vision #652 and roadmap #653. + +Unknown class names are denied. The coordinator requires both the class +permission and an eligible request role. Approval gates are additional: a +caller cannot turn a request permission into execution authority. + +| Restart class | Required permission | Expected blast radius | Drain requirement | Approval requirement | Audit requirement | Recovery behavior | +|---|---|---|---|---|---|---| +| `client_reconnect` | `mcp.reconnect.client` | none | none | self service | class, actor, client namespace, reason, outcome | Reconnect only the caller's client transport. No daemon or peer work changes. | +| `session_reconnect` | `mcp.reconnect.session` | low | requesting-session safe point | self service | class, actor, session, reason, outcome | Rebind identity, capability, and workspace state for one session. | +| `worker_restart` | `mcp.restart.worker.request` | low | target worker | controller approval + automated gates | class, actor, worker, approval, scoped drain, outcome | Restart one worker after its own leases and mutations drain. | +| `role_runtime_restart` | `mcp.restart.role_runtime.request` | medium | target role runtime | controller approval + automated gates | class, actor, role namespace, approval, scoped drain, outcome | Restart and re-probe one role runtime; unrelated roles remain available. | +| `connector_restart` | `mcp.restart.connector.request` | medium | target connector | controller approval + automated gates | class, actor, connector, approval, scoped drain, outcome | Restart one connector while unrelated runtimes remain available. | +| `configuration_reload` | `mcp.reload.configuration.request` | low | mutation quiesce | controller approval + automated gates | class, actor, configuration revision, approval, outcome | Gracefully reload configuration without replacing the daemon. | +| `rolling_mcp_restart` | `mcp.restart.rolling.request` | medium | one instance at a time | controller approval + automated gates | class, actor, instance order, approval, per-instance drains, outcome | Drain, restart, verify, and restore each instance before advancing. | +| `full_mcp_restart` | `mcp.restart.full.request` | high | all sessions and mutations | controller approval + automated gates | class, actor, full impact, approval, full drain proof, outcome | Replace the complete MCP runtime only after a verified full drain. | +| `host_restart` | `mcp.restart.host.request` | high | all host work | controller approval + infrastructure operator | class, actor, host/change or incident id, approval, full drain proof, outcome | Hand off to infrastructure ownership and reconcile every runtime afterward. | + +## Drain boundary + +Only `full_mcp_restart` and `host_restart` set `full_drain_required=true`. +Reconnects and configuration reloads do not disrupt peer sessions. Worker, +role-runtime, and connector restarts evaluate only their explicitly named +target. Rolling restart drains one instance at a time. Missing required target +scope denies the request rather than silently widening it to a full restart. + +## Permission and approval boundary + +Author, reviewer, merger, and reconciler roles may self-request reconnects and +request scoped worker/role/connector/reload recovery. They cannot request +rolling, full, or host restart classes. Controller/operator/admin roles may +request the broader classes, while execution remains operator/admin-owned. +Controller approval is independently required for every class above a session +reconnect. Host restart additionally requires infrastructure-operator proof. + +The MCP request tool derives class permissions from its authenticated runtime +role. It does not accept caller-supplied permissions. Controller and operator +authorization are read from the already-running daemon environment, never +from a request argument. + +## Audit and failure behavior + +Every impact audit and every console restart/reload audit includes a +`restart_class` field. The impact audit also includes the exact +`required_permission`. Unknown classes, missing permissions, ineligible roles, +missing approval, missing scoped targets, and incomplete inventory all deny +fail closed. Manual process kills remain forbidden and contaminating (#630). diff --git a/docs/mcp-restart-coordinator.md b/docs/mcp-restart-coordinator.md index b1368c0..628895f 100644 --- a/docs/mcp-restart-coordinator.md +++ b/docs/mcp-restart-coordinator.md @@ -6,11 +6,23 @@ console (#642 / #652) can see the blast radius *before* concurrent LLM work is disrupted. Uncoordinated restarts destroy in-flight author/reviewer/merger work and give operators no way to see what they are about to break. -This lands the coordinator + impact DTO + a dry-run MCP tool. It is the single +This lands the coordinator + impact DTO + the MCP tool. It is the single sanctioned entry point for restart evaluation post-#657 (which inventoried the -restart/reload/kill paths). The **mutative apply** path — actually performing a -restart — is a later child gated by a drain proof and is explicitly out of -scope here. +restart/reload/kill paths). + +The **drain-proof hard gate now executes inside this tool** (#661, via PR #882): +an apply request (`dry_run=False`) is evaluated against a drain proof here and +denied when that proof is missing, expired, unclean, tampered with, or stale. +It is no longer a separate child operation. What remains a later child is only +the **execution** step — actually stopping and restoring a process. This tool +still never restarts anything: `apply_supported` is always `false` and +`restart_performed` is always `false`. + +The coordinator now routes every request through the restart-class policy +matrix defined for #663. See +[`mcp-restart-classes.md`](./mcp-restart-classes.md) for permissions, expected +blast radius, scoped drain and approval requirements, audit fields, and +recovery behavior for all nine classes. ## Components @@ -19,7 +31,8 @@ scope here. | `restart_coordinator.evaluate_restart_impact` | `restart_coordinator.py` | Pure classification: inventory → impact report DTO. No I/O, no restart. | | `RestartImpactReport` / `SessionImpact` / `LeaseImpact` | `restart_coordinator.py` | Console-facing DTO (`.as_dict()` is JSON-serializable). | | `ControlPlaneDB.list_sessions` | `control_plane_db.py` | Read-only session inventory (the process-level unit a restart kills). | -| `gitea_request_mcp_restart` | `gitea_mcp_server.py` | MCP tool: gathers inventory from the #613 DB, calls the coordinator, returns the report. Dry-run only. | +| `gitea_request_mcp_restart` | `gitea_mcp_server.py` | MCP tool: gathers inventory from the #613 DB, calls the coordinator, returns the report, and on `dry_run=False` runs the #661 drain-proof hard gate. Never restarts a process. | +| `drain_proof.gate_apply_restart` | `drain_proof.py` | The #661 hard gate: verifies a drain proof against the current impact fingerprint, or records an authorized break-glass bypass. | ## Dimensions evaluated @@ -77,17 +90,61 @@ authorization is present. ```text gitea_request_mcp_restart(remote, host, org, repo, dry_run=True, request_override=False, - session_id=None, limit=200) + session_id=None, limit=200, + restart_class="full_mcp_restart", + target_session_id=None, target_role=None, + target_connector=None, + drain_proof_json=None, + request_break_glass=False) ``` -Read-only, dry-run, and it **never restarts anything**. `apply_supported` is -always `false`; passing `dry_run=False` performs no restart and reports that -apply is gated by a drain proof (a separate child). +It **never restarts anything**: `apply_supported` is always `false` and +`restart_performed` is always `false`. + +### Dry-run versus apply + +| Call | Behavior | +|------|----------| +| `dry_run=True` (default) | Read-only impact preview. No drain proof is required or consulted. | +| `dry_run=False` | The #661 drain-proof hard gate runs **in this tool**. The outcome is reported under `apply_gate` / `apply_authorized`; a denial also returns a durable `incident` descriptor. Still no restart. | + +### Authorization ordering + +An apply requires **both** authorizations, and they are independent: + +1. **Restart-class authorization** (#663) — the requester's role and permissions + must allow the requested class, the class's approval requirement must be + satisfied, and any target-scoped class must name its target. Failing any of + these makes `allow_restart` `false`. +2. **Drain-proof gate** (#661) — a valid, unexpired, clean proof bound to the + current impact fingerprint, or an authorized break-glass. + +`apply_authorized` is the conjunction: `gate.allow and allow_restart`. A clean +drain proof therefore cannot override a class or requester-role denial, and a +denied class never reports an authorized apply. `apply_gate` carries +`drain_gate_allow` and `restart_class_authorized` so a denial is attributable to +the authorization that produced it. + +### Break-glass + +Break-glass bypasses the **drain proof only** — never the restart-class matrix. +It is honoured solely when `request_break_glass` is set *and* the environment +carries `GITEA_BREAKGLASS_RESTART_AUTHORIZATION`; like operator override, the +tool argument expresses caller intent and cannot be self-asserted by a worker +session. `break_glass_requested` and `break_glass_authorized` are both reported, +so a bypass is never silent. + +### Fail closed on apply + +A missing, malformed, expired, unclean, tampered, or fingerprint-stale drain +proof denies the apply and returns an `incident` descriptor. An unknown restart +class denies before any of this. Ambiguity always denies. ## Audit Every evaluation carries an `audit_record` (event, coordinator version, verdict, -allow decision, blast radius, counts, timestamp) so restart decisions are +restart class, required permission, allow decision, blast radius, counts, +timestamp) so restart decisions are auditable. No secrets flow through the coordinator — session ids, pids, and profiles are operational metadata only. diff --git a/docs/webui-local-dev.md b/docs/webui-local-dev.md index d9cc022..3e01401 100644 --- a/docs/webui-local-dev.md +++ b/docs/webui-local-dev.md @@ -77,7 +77,9 @@ status, onboarding checklist state, and the fail-closed error payloads (#635). | `/api/actions/{id}/preview` | Mutation ledger preview (GET, read-only) | | `/leases` | Lease and collision visibility (#433) | | `/api/leases` | JSON lease/collision export | -| `/sessions` | Phase 1 shell stub — session inventory (backed by #636) | +| `/sessions` | Runtime and session view (#641) — health + inventory sessions/namespaces/worktrees | +| `/api/sessions` | JSON export for the runtime/session view | +| `/api/v1/sessions` | Versioned alias of `/api/sessions` | | `/inventory` | Phase 1 shell stub — unified inventory (backed by #636) | | `/timeline` | Phase 1 shell stub — workflow event timeline | | `/policy` | Phase 1 shell stub — capability/role policy placeholder | @@ -284,11 +286,46 @@ The header carries two read-only status badges — an **environment** badge a **mode: read-only** badge — plus a **Docs** link to this document. No privileged action controls are present in the Phase 1 shell. -Not-yet-implemented surfaces (`/sessions`, `/inventory`, `/timeline`, -`/policy`, `/insights`) resolve to graceful read-only stub pages instead of -404s; their backing views land in later child issues of #631 (the inventory -surfaces are backed by #636). Mutating methods on stub routes still fail closed -with `read-only-mvp`. +Not-yet-implemented surfaces (`/inventory`, `/timeline`, `/policy`, +`/insights`) resolve to graceful read-only stub pages instead of 404s; their +backing views land in later child issues of #631 (the inventory surfaces are +backed by #636). Mutating methods on stub routes still fail closed with +`read-only-mvp`. + +### Runtime and sessions (#641) + +`/sessions` is a live Phase 1 read-only view that composes: + +* runtime health from `#430` (profile, role, identity, master parity, stale warning) +* control-plane sessions / leases and filesystem locks / worktrees / namespaces from `#636` +* durable contamination markers when detectable (`#630` runtime recovery, `#671` stable-branch push) + +It surfaces stale indicators (dead PID, expired lease) and never silences an +active contamination marker. Recovery links point only at sanctioned +reconnect/operator restart docs (`docs/mcp-namespace-eof-recovery.md`, +`docs/mcp-namespace-health.md`, `docs/mcp-restart-path-inventory.md`, this +document). The page does **not** restart, kill, or take over sessions; manual +`pkill` of MCP daemons is contamination, not recovery. + +Honesty rules specific to this view: + +* **Ownership columns never assert absence they cannot prove.** When the + `leases` or `locks` section is degraded or unavailable, the Leases and + Worktree-binding cells render `unknown (inventory )` with an + *authority unproven* badge instead of `none` / `unbound`, and a caveat names + the unreadable sections. A worktree binding is correlated through lease work + numbers, so it is unproven when *either* section fails to read. + `/api/sessions` carries the same facts as `ownership_authority_complete`, + `ownership_section_status`, and per-row `lease_authority` / + `worktree_authority`, so a JSON consumer can tell "holds none" from "could + not be read". +* **Contamination text is redacted at the display boundary.** Marker payloads + (`command_summary`, `reason_class`, `session_id`, `role`) are + operator-supplied free text that does not arrive through inventory scrubbing, + so they pass through `webui.inventory.scrub_text`, which collapses `$HOME` and + redacts credential-shaped tokens and URL userinfo *anywhere* in the string. + The write-time redactor is a narrow denylist and is not relied on. The field + itself is kept — it is the `#630` evidence naming which daemon was killed. ## System-health dashboard (#639) diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index 91f1b3a..aab8249 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -22569,12 +22569,17 @@ def gitea_request_mcp_restart( request_override: bool = False, session_id: str | None = None, limit: int = 200, + restart_class: str = "full_mcp_restart", + target_session_id: str | None = None, + target_role: str | None = None, + target_connector: str | None = None, drain_proof_json: str | None = None, request_break_glass: bool = False, ) -> dict: """Evaluate a proposed MCP restart and return an impact preview (#658). - Central restart coordinator: gathers live control-plane state (sessions, + Central restart coordinator: resolves the requested restart class, gathers + live control-plane state (sessions, leases/locks, in-flight issue/PR work, mutations, worktrees) and returns a blast-radius impact report with a ``safe`` / ``unsafe`` / ``override`` verdict, so the console (#642/#652) and operators can see what a restart @@ -22588,7 +22593,16 @@ def gitea_request_mcp_restart( only when ``request_break_glass`` is set *and* the environment carries ``GITEA_BREAKGLASS_RESTART_AUTHORIZATION``. Even an authorized gate performs no restart here; actual execution is a further child. The gate outcome is - reported under ``apply_gate`` / ``apply_authorized``. + reported under ``apply_gate``. + + ``apply_authorized`` requires **both** authorizations to pass: the #661 drain + gate *and* the #663 restart-class matrix (``allow_restart``). They are + independent — the drain gate proves the blast radius was drained and knows + nothing about whether this requester may request this class — so a class the + matrix denied never reports an authorized apply. Break-glass bypasses the + drain proof only; it never bypasses the class matrix. ``apply_gate`` carries + ``drain_gate_allow`` and ``restart_class_authorized`` so a denial is + attributable to the authorization that produced it. Operator override authority is read from the process environment (``GITEA_OPERATOR_RESTART_OVERRIDE_AUTHORIZATION``), never self-asserted by @@ -22673,6 +22687,9 @@ def gitea_request_mcp_restart( profile = get_profile() profile_name = (profile.get("profile_name") or "").strip() or "session" + requester_role = ( + profile.get("role_kind") or profile.get("role") or "" + ).strip().lower() sid = (session_id or "").strip() or f"{profile_name}-{os.getpid()}" # Override authority is read from the environment only — a worker session @@ -22682,6 +22699,15 @@ def gitea_request_mcp_restart( (os.environ.get("GITEA_OPERATOR_RESTART_OVERRIDE_AUTHORIZATION") or "").strip() ) operator_override = bool(request_override and operator_authorized) + controller_approved = bool( + ( + os.environ.get("GITEA_CONTROLLER_RESTART_APPROVAL_AUTHORIZATION") + or "" + ).strip() + ) + requester_permissions = restart_coordinator.permissions_for_role( + requester_role + ) inventory = { "sessions": sessions, @@ -22696,6 +22722,14 @@ def gitea_request_mcp_restart( operator_override=operator_override, requesting_session_id=sid, dry_run=True, # coordinator is always analysis-only (#658) + restart_class=restart_class, + requester_role=requester_role, + requester_permissions=requester_permissions, + controller_approved=controller_approved, + operator_authorized=operator_authorized, + target_session_id=target_session_id, + target_role=target_role, + target_connector=target_connector, ) payload = report.as_dict() @@ -22707,6 +22741,9 @@ def gitea_request_mcp_restart( payload["requesting_session_id"] = sid payload["operator_override_requested"] = bool(request_override) payload["operator_override_authorized"] = operator_authorized + payload["controller_approval_authorized"] = controller_approved + payload["requester_role"] = requester_role + payload["requester_permissions"] = list(requester_permissions) # Actual restart execution remains a further child; this tool never restarts # a process. What #661 adds is the *hard gate*: an apply request (dry_run # False) must present a valid, unexpired, clean drain proof, or it is denied @@ -22744,8 +22781,25 @@ def gitea_request_mcp_restart( gate_payload["reasons"] = [proof_parse_error] + list( gate_payload.get("reasons") or [] ) + # The #663 restart-class matrix and the #661 drain gate are two + # independent authorizations, and an apply requires BOTH. ``gate.allow`` + # proves only that the blast radius was drained — or that break-glass + # was authorized — and knows nothing about whether this requester may + # request this class at all. Conjoining them keeps a class the matrix + # denied from ever reporting an authorized apply, and keeps break-glass + # scoped to what it is for: bypassing the drain proof, never the + # least-privilege class matrix. + restart_class_authorized = bool(report.allow_restart) + gate_payload["drain_gate_allow"] = bool(gate.allow) + gate_payload["restart_class_authorized"] = restart_class_authorized + if not restart_class_authorized: + gate_payload["reasons"] = list(gate_payload.get("reasons") or []) + [ + "restart class authorization denied; apply denied regardless of " + "drain proof or break-glass (fail closed, #663)", + *(report.authorization_reasons or []), + ] payload["apply_gate"] = gate_payload - payload["apply_authorized"] = gate.allow + payload["apply_authorized"] = bool(gate.allow and restart_class_authorized) payload["break_glass_requested"] = bool(request_break_glass) payload["break_glass_authorized"] = break_glass_authorized # Even an authorized gate performs no restart here: execution is a later diff --git a/restart_coordinator.py b/restart_coordinator.py index 5db7de0..5452ad0 100644 --- a/restart_coordinator.py +++ b/restart_coordinator.py @@ -28,11 +28,12 @@ from __future__ import annotations from dataclasses import dataclass, field from datetime import datetime, timezone +from enum import Enum from typing import Any, Mapping, Sequence import lease_lifecycle -COORDINATOR_VERSION = "1.0.0-issue-658" +COORDINATOR_VERSION = "1.1.0-issue-663" # Restart verdicts. Exactly the three the acceptance criteria name. VERDICT_SAFE = "safe" @@ -54,6 +55,194 @@ LEASE_FRESHNESS_LIVE = "active" DEFAULT_SESSION_HEARTBEAT_STALE_SECONDS = 900 +class RestartClass(str, Enum): + """The only restart/recovery classes accepted by the coordinator.""" + + CLIENT_RECONNECT = "client_reconnect" + SESSION_RECONNECT = "session_reconnect" + WORKER_RESTART = "worker_restart" + ROLE_RUNTIME_RESTART = "role_runtime_restart" + CONNECTOR_RESTART = "connector_restart" + CONFIGURATION_RELOAD = "configuration_reload" + ROLLING_MCP_RESTART = "rolling_mcp_restart" + FULL_MCP_RESTART = "full_mcp_restart" + HOST_RESTART = "host_restart" + + +@dataclass(frozen=True) +class RestartClassPolicy: + """Least-privilege policy for one :class:`RestartClass`.""" + + restart_class: RestartClass + required_permission: str + expected_blast_radius: str + drain_requirement: str + full_drain_required: bool + approval_requirement: str + audit_requirement: str + recovery_behavior: str + request_roles: tuple[str, ...] + execution_roles: tuple[str, ...] + + def as_dict(self) -> dict[str, Any]: + return { + "restart_class": self.restart_class.value, + "required_permission": self.required_permission, + "expected_blast_radius": self.expected_blast_radius, + "drain_requirement": self.drain_requirement, + "full_drain_required": self.full_drain_required, + "approval_requirement": self.approval_requirement, + "audit_requirement": self.audit_requirement, + "recovery_behavior": self.recovery_behavior, + "request_roles": list(self.request_roles), + "execution_roles": list(self.execution_roles), + } + + +WORKER_ROLES = ("author", "reviewer", "merger", "reconciler") +CONTROL_ROLES = ("controller", "operator", "admin") +ALL_REQUEST_ROLES = WORKER_ROLES + CONTROL_ROLES + +RESTART_CLASS_POLICIES: dict[RestartClass, RestartClassPolicy] = { + RestartClass.CLIENT_RECONNECT: RestartClassPolicy( + RestartClass.CLIENT_RECONNECT, + "mcp.reconnect.client", + BLAST_NONE, + "none", + False, + "self_service", + "record class, actor, client namespace, reason, and outcome", + "Reconnect only the caller's client transport; no daemon or peer session changes.", + ALL_REQUEST_ROLES, + ALL_REQUEST_ROLES, + ), + RestartClass.SESSION_RECONNECT: RestartClassPolicy( + RestartClass.SESSION_RECONNECT, + "mcp.reconnect.session", + BLAST_LOW, + "requesting_session_safe_point", + False, + "self_service", + "record class, actor, session id, reason, and outcome", + "Rebind identity, capability, and workspace state for one session.", + ALL_REQUEST_ROLES, + ALL_REQUEST_ROLES, + ), + RestartClass.WORKER_RESTART: RestartClassPolicy( + RestartClass.WORKER_RESTART, + "mcp.restart.worker.request", + BLAST_LOW, + "target_worker", + False, + "controller_approval_and_automated_gates", + "record class, actor, target worker, approval, drain proof, and outcome", + "Restart one worker after its own lease and mutation scope is drained.", + ALL_REQUEST_ROLES, + ("operator", "admin"), + ), + RestartClass.ROLE_RUNTIME_RESTART: RestartClassPolicy( + RestartClass.ROLE_RUNTIME_RESTART, + "mcp.restart.role_runtime.request", + BLAST_MEDIUM, + "target_role_runtime", + False, + "controller_approval_and_automated_gates", + "record class, actor, role namespace, approval, drain proof, and outcome", + "Restart only the selected role runtime and then re-probe that namespace.", + ALL_REQUEST_ROLES, + ("operator", "admin"), + ), + RestartClass.CONNECTOR_RESTART: RestartClassPolicy( + RestartClass.CONNECTOR_RESTART, + "mcp.restart.connector.request", + BLAST_MEDIUM, + "target_connector", + False, + "controller_approval_and_automated_gates", + "record class, actor, connector id, approval, drain proof, and outcome", + "Restart one connector while unrelated role runtimes remain available.", + ALL_REQUEST_ROLES, + ("operator", "admin"), + ), + RestartClass.CONFIGURATION_RELOAD: RestartClassPolicy( + RestartClass.CONFIGURATION_RELOAD, + "mcp.reload.configuration.request", + BLAST_LOW, + "mutation_quiesce", + False, + "controller_approval_and_automated_gates", + "record class, actor, configuration revision, approval, and outcome", + "Gracefully reload configuration without replacing the daemon process.", + ALL_REQUEST_ROLES, + ("operator", "admin"), + ), + RestartClass.ROLLING_MCP_RESTART: RestartClassPolicy( + RestartClass.ROLLING_MCP_RESTART, + "mcp.restart.rolling.request", + BLAST_MEDIUM, + "one_instance_at_a_time", + False, + "controller_approval_and_automated_gates", + "record class, actor, instance order, approval, per-instance drains, and outcome", + "Drain, restart, verify, and restore one instance before advancing to the next.", + CONTROL_ROLES, + ("operator", "admin"), + ), + RestartClass.FULL_MCP_RESTART: RestartClassPolicy( + RestartClass.FULL_MCP_RESTART, + "mcp.restart.full.request", + BLAST_HIGH, + "all_sessions_and_mutations", + True, + "controller_approval_and_automated_gates", + "record class, actor, full impact report, approval, drain proof, and outcome", + "Stop and restore the complete MCP runtime only after a verified full drain.", + CONTROL_ROLES, + ("operator", "admin"), + ), + RestartClass.HOST_RESTART: RestartClassPolicy( + RestartClass.HOST_RESTART, + "mcp.restart.host.request", + BLAST_HIGH, + "all_host_work", + True, + "controller_approval_plus_infrastructure_operator", + "record class, actor, host, incident or change id, approval, drain proof, and outcome", + "Hand off to infrastructure ownership; reconcile every runtime after the host returns.", + ("controller", "operator", "admin"), + ("operator", "admin"), + ), +} + + +def resolve_restart_class(value: RestartClass | str) -> RestartClass: + """Resolve a restart class or fail closed for an unknown value.""" + + if isinstance(value, RestartClass): + return value + try: + return RestartClass(str(value).strip()) + except ValueError as exc: + raise ValueError(f"unknown restart class {value!r}; deny (fail closed)") from exc + + +def restart_class_policy(value: RestartClass | str) -> RestartClassPolicy: + """Return the canonical policy for *value*.""" + + return RESTART_CLASS_POLICIES[resolve_restart_class(value)] + + +def permissions_for_role(role: str | None) -> tuple[str, ...]: + """Return request permissions granted to a workflow role by this policy.""" + + normalized = str(role or "").strip().lower() + return tuple( + policy.required_permission + for policy in RESTART_CLASS_POLICIES.values() + if normalized in policy.request_roles + ) + + def _utc_now() -> datetime: return datetime.now(timezone.utc) @@ -75,6 +264,7 @@ class SessionImpact: heartbeat_stale: bool is_requester: bool live: bool + connector: str | None = None def as_dict(self) -> dict[str, Any]: return { @@ -87,6 +277,7 @@ class SessionImpact: "heartbeat_stale": self.heartbeat_stale, "is_requester": self.is_requester, "live": self.live, + "connector": self.connector, } @@ -105,6 +296,7 @@ class LeaseImpact: disruptive: bool is_mutation: bool is_critical_section: bool + connector: str | None = None def as_dict(self) -> dict[str, Any]: return { @@ -119,6 +311,7 @@ class LeaseImpact: "disruptive": self.disruptive, "is_mutation": self.is_mutation, "is_critical_section": self.is_critical_section, + "connector": self.connector, } @@ -127,6 +320,13 @@ class RestartImpactReport: """Impact preview DTO returned to the console / operator (#642/#652).""" coordinator_version: str + restart_class: str + restart_policy: dict[str, Any] + policy_enforced: bool + permission_authorized: bool + role_authorized: bool + approval_satisfied: bool + authorization_reasons: list[str] evaluated_at: str dry_run: bool restart_performed: bool @@ -153,6 +353,13 @@ class RestartImpactReport: def as_dict(self) -> dict[str, Any]: return { "coordinator_version": self.coordinator_version, + "restart_class": self.restart_class, + "restart_policy": dict(self.restart_policy), + "policy_enforced": self.policy_enforced, + "permission_authorized": self.permission_authorized, + "role_authorized": self.role_authorized, + "approval_satisfied": self.approval_satisfied, + "authorization_reasons": list(self.authorization_reasons), "evaluated_at": self.evaluated_at, "dry_run": self.dry_run, "restart_performed": self.restart_performed, @@ -206,6 +413,7 @@ def _classify_session( requesting_session_id and session_id == requesting_session_id ), live=live, + connector=(str(row.get("connector") or "").strip() or None), ) @@ -258,6 +466,7 @@ def _classify_lease(row: Mapping[str, Any]) -> LeaseImpact: disruptive=disruptive, is_mutation=is_mutation, is_critical_section=disruptive, + connector=(str(row.get("connector") or "").strip() or None), ) @@ -279,6 +488,14 @@ def evaluate_restart_impact( requesting_session_id: str | None = None, dry_run: bool = True, session_heartbeat_stale_seconds: int = DEFAULT_SESSION_HEARTBEAT_STALE_SECONDS, + restart_class: RestartClass | str | None = None, + requester_role: str | None = None, + requester_permissions: Sequence[str] | None = None, + controller_approved: bool = False, + operator_authorized: bool = False, + target_session_id: str | None = None, + target_role: str | None = None, + target_connector: str | None = None, ) -> RestartImpactReport: """Evaluate a proposed MCP restart and return an impact preview. @@ -301,6 +518,61 @@ def evaluate_restart_impact( """ moment = now or _utc_now() reasons: list[str] = [] + authorization_reasons: list[str] = [] + + # ``None`` preserves the pre-#663 impact-only API for callers that have not + # yet been migrated. All MCP requests pass an explicit class and therefore + # take the fail-closed policy path. + policy_enforced = restart_class is not None + try: + resolved_class = resolve_restart_class( + restart_class or RestartClass.FULL_MCP_RESTART + ) + policy = RESTART_CLASS_POLICIES[resolved_class] + unknown_class = False + except ValueError as exc: + resolved_class = None + policy = None + unknown_class = True + authorization_reasons.append(str(exc)) + + normalized_role = str(requester_role or "").strip().lower() + granted = {str(p).strip() for p in (requester_permissions or ())} + if policy_enforced and policy is not None: + permission_authorized = policy.required_permission in granted + role_authorized = normalized_role in policy.request_roles + if not permission_authorized: + authorization_reasons.append( + f"missing required permission {policy.required_permission!r}" + ) + if not role_authorized: + authorization_reasons.append( + f"role {normalized_role or 'unknown'!r} may not request " + f"{policy.restart_class.value}" + ) + elif unknown_class: + permission_authorized = False + role_authorized = False + else: + permission_authorized = True + role_authorized = True + + if policy_enforced and policy is not None: + approval = policy.approval_requirement + if approval == "self_service": + approval_satisfied = True + elif approval == "controller_approval_plus_infrastructure_operator": + approval_satisfied = bool(controller_approved and operator_authorized) + else: + approval_satisfied = bool(controller_approved) + if not approval_satisfied: + authorization_reasons.append( + f"approval requirement not satisfied: {approval}" + ) + elif unknown_class: + approval_satisfied = False + else: + approval_satisfied = True inventory_complete = bool(inventory.get("inventory_complete", False)) incomplete_reasons = [str(r) for r in (inventory.get("incomplete_reasons") or [])] @@ -323,15 +595,67 @@ def evaluate_restart_impact( ] lease_impacts = [_classify_lease(l) for l in leases_raw] - # Only *other* live sessions and live leases constitute blast radius: a - # restart that would kill only the requesting session with no other work in - # flight is safe. + # Route impact through the selected class. Narrow classes never inherit a + # full-runtime drain merely because unrelated work exists. + target_complete = True + if resolved_class in { + RestartClass.CLIENT_RECONNECT, + RestartClass.SESSION_RECONNECT, + RestartClass.CONFIGURATION_RELOAD, + }: + scoped_sessions: list[SessionImpact] = [] + scoped_leases: list[LeaseImpact] = [] + elif resolved_class == RestartClass.WORKER_RESTART: + selected_session = (target_session_id or "").strip() + target_complete = bool(selected_session) + scoped_sessions = [ + s for s in session_impacts if s.session_id == selected_session + ] + scoped_leases = [ + l for l in lease_impacts if l.session_id == selected_session + ] + elif resolved_class == RestartClass.ROLE_RUNTIME_RESTART: + selected_role = (target_role or "").strip().lower() + target_complete = bool(selected_role) + scoped_sessions = [ + s for s in session_impacts if str(s.role or "").lower() == selected_role + ] + scoped_leases = [ + l for l in lease_impacts if str(l.role or "").lower() == selected_role + ] + elif resolved_class == RestartClass.CONNECTOR_RESTART: + selected_connector = (target_connector or "").strip() + target_complete = bool(selected_connector) + scoped_sessions = [ + s for s in session_impacts if s.connector == selected_connector + ] + scoped_leases = [ + l for l in lease_impacts if l.connector == selected_connector + ] + else: + scoped_sessions = list(session_impacts) + scoped_leases = list(lease_impacts) + + if policy_enforced and not target_complete: + authorization_reasons.append( + f"target required for {resolved_class.value if resolved_class else 'unknown class'}" + ) + other_live_sessions = [ - s for s in session_impacts if s.live and not s.is_requester + s for s in scoped_sessions if s.live and not s.is_requester ] - disruptive_leases = [l for l in lease_impacts if l.disruptive] - critical_sections = [l for l in lease_impacts if l.is_critical_section] - mutations = [l for l in lease_impacts if l.is_mutation] + disruptive_leases = [l for l in scoped_leases if l.disruptive] + critical_sections = [l for l in scoped_leases if l.is_critical_section] + mutations = [l for l in scoped_leases if l.is_mutation] + terminal_lock_in_scope = ( + terminal_lock + if resolved_class + not in { + RestartClass.CLIENT_RECONNECT, + RestartClass.SESSION_RECONNECT, + } + else None + ) affected_issues = sorted( { @@ -348,9 +672,24 @@ def evaluate_restart_impact( } ) - disruptive = bool(disruptive_leases or other_live_sessions or terminal_lock) + disruptive = bool( + disruptive_leases or other_live_sessions or terminal_lock_in_scope + ) - if not inventory_complete: + authorization_ok = bool( + not unknown_class + and permission_authorized + and role_authorized + and approval_satisfied + and target_complete + ) + + if policy_enforced and not authorization_ok: + verdict = VERDICT_UNSAFE + allow_restart = False + reasons.append("restart class authorization denied (fail closed)") + reasons.extend(authorization_reasons) + elif not inventory_complete: verdict = VERDICT_UNSAFE allow_restart = False reasons.append( @@ -381,7 +720,7 @@ def evaluate_restart_impact( f"{len(critical_sections)} critical section(s) in flight " "(active lease with a live owner)" ) - if terminal_lock: + if terminal_lock_in_scope: reasons.append("active terminal (merge) lock present") override_would_allow = bool(inventory_complete and disruptive) @@ -411,6 +750,12 @@ def evaluate_restart_impact( audit_record = { "event": "restart_impact_evaluated", "coordinator_version": COORDINATOR_VERSION, + "restart_class": ( + resolved_class.value if resolved_class else str(restart_class or "") + ), + "required_permission": ( + policy.required_permission if policy is not None else None + ), "evaluated_at": moment.isoformat(), "dry_run": dry_run, "operator_override": bool(operator_override), @@ -424,6 +769,15 @@ def evaluate_restart_impact( return RestartImpactReport( coordinator_version=COORDINATOR_VERSION, + restart_class=( + resolved_class.value if resolved_class else str(restart_class or "") + ), + restart_policy=policy.as_dict() if policy is not None else {}, + policy_enforced=policy_enforced, + permission_authorized=permission_authorized, + role_authorized=role_authorized, + approval_satisfied=approval_satisfied, + authorization_reasons=authorization_reasons, evaluated_at=moment.isoformat(), dry_run=dry_run, restart_performed=False, @@ -440,9 +794,11 @@ def evaluate_restart_impact( affected_issues=affected_issues, affected_prs=affected_prs, mutations=mutations, - terminal_lock=dict(terminal_lock) - if isinstance(terminal_lock, Mapping) - else terminal_lock, + terminal_lock=( + dict(terminal_lock_in_scope) + if isinstance(terminal_lock_in_scope, Mapping) + else terminal_lock_in_scope + ), ack_state=ack_state, prior_recovery_attempts=prior_recovery_attempts, counts=counts, diff --git a/tests/test_issue_886_apply_authorization_conjunction.py b/tests/test_issue_886_apply_authorization_conjunction.py new file mode 100644 index 0000000..6fa2888 --- /dev/null +++ b/tests/test_issue_886_apply_authorization_conjunction.py @@ -0,0 +1,381 @@ +"""``apply_authorized`` requires BOTH authorizations (#886 review blocker B1). + +The #663 restart-class matrix and the #661 drain-proof hard gate are independent +authorizations that first coexisted when PR #882 landed on master and PR #886 +merged it into the restart-class branch. The union preserved both, but the apply +decision consulted only the drain gate:: + + payload["apply_authorized"] = gate.allow # pre-fix + +so a clean drain proof — or an authorized break-glass, which needs no proof at +all — reported ``apply_authorized: True`` for a restart class the least-privilege +matrix had just denied, in the same payload that carried +``allow_restart: False`` and "role 'author' may not request full_mcp_restart". + +These tests pin the conjunction and the properties that must survive it. They +exercise the real MCP tool, which previously had no test coverage at all — that +absence is why the defect shipped. +""" + +from __future__ import annotations + +import json +import os +import unittest +from unittest.mock import patch + +import drain_proof +import gitea_mcp_server as srv + +CONTROLLER_APPROVAL_ENV = "GITEA_CONTROLLER_RESTART_APPROVAL_AUTHORIZATION" +BREAK_GLASS_ENV = "GITEA_BREAKGLASS_RESTART_AUTHORIZATION" + +# A quiet control plane: nothing live, so the blast radius never masks the +# authorization outcome under test. +QUIET_SESSIONS: list[dict] = [] +QUIET_LEASES: list[dict] = [] + + +class _FakeDB: + """Minimal control-plane DB stand-in for the restart inventory.""" + + def __init__(self, sessions=QUIET_SESSIONS, terminal=None): + self._sessions = list(sessions) + self._terminal = terminal + + def list_sessions(self, statuses=None, limit=None): + return list(self._sessions) + + def get_active_terminal_lock(self, remote=None, org=None, repo=None): + return self._terminal + + +def _profile(role: str) -> dict: + return {"profile_name": f"prgs-{role}", "role_kind": role, "role": role} + + +class _RestartToolHarness(unittest.TestCase): + """Drives the real ``gitea_request_mcp_restart`` with a stubbed inventory.""" + + def _call(self, *, role: str, env: dict | None = None, **kwargs) -> dict: + environ = {k: v for k, v in os.environ.items() + if k not in (CONTROLLER_APPROVAL_ENV, BREAK_GLASS_ENV)} + environ.update(env or {}) + with patch.object(srv, "_profile_operation_gate", return_value=None), \ + patch.object(srv, "_resolve", + return_value=("gitea.prgs.cc", + "Scaled-Tech-Consulting", + "Gitea-Tools")), \ + patch.object(srv, "get_profile", return_value=_profile(role)), \ + patch.object(srv, "_control_plane_db_or_error", + return_value=(_FakeDB(), [])), \ + patch.object(srv.lease_lifecycle, "list_active_leases", + return_value={"leases": list(QUIET_LEASES)}), \ + patch.dict(os.environ, environ, clear=True): + return srv.gitea_request_mcp_restart( + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + session_id="probe-session", + **kwargs, + ) + + def _clean_proof_for(self, preview: dict) -> str: + """Mint a genuinely clean, signature-valid proof bound to *preview*. + + Built from the tool's own dry-run report, so the fingerprint matches and + the proof is rejected for authorization reasons only — never because it + was stale or forged. + """ + proof = drain_proof.build_drain_proof( + impact_report=preview, + drain_state={ + "assignments_stopped": True, + "checkpoints_complete": True, + "handoffs_verified": True, + "leases_handled": True, + "acks": {}, + }, + requesting_session_id="probe-session", + ) + self.assertTrue(proof.clean, "harness must mint a clean proof") + return json.dumps(proof.as_dict()) + + +class TestConjunction(_RestartToolHarness): + """AC1/AC2 — the two authorizations are ANDed, in both directions.""" + + def test_gate_allow_with_class_denied_yields_apply_authorized_false(self): + # An author may not request full_mcp_restart (CONTROL_ROLES only). + preview = self._call(role="author", restart_class="full_mcp_restart") + self.assertFalse(preview["allow_restart"]) + + result = self._call( + role="author", + restart_class="full_mcp_restart", + dry_run=False, + drain_proof_json=self._clean_proof_for(preview), + ) + + self.assertTrue(result["apply_gate"]["drain_gate_allow"], + "drain gate itself should have allowed this proof") + self.assertFalse(result["apply_gate"]["restart_class_authorized"]) + self.assertFalse(result["apply_authorized"], + "a clean proof must not authorize a denied class") + self.assertFalse(result["allow_restart"]) + + def test_gate_allow_with_class_allowed_can_yield_apply_authorized_true(self): + preview = self._call( + role="operator", + restart_class="full_mcp_restart", + env={CONTROLLER_APPROVAL_ENV: "operator-approved"}, + ) + self.assertTrue(preview["allow_restart"], + "operator + controller approval must authorize the class") + + result = self._call( + role="operator", + restart_class="full_mcp_restart", + dry_run=False, + drain_proof_json=self._clean_proof_for(preview), + env={CONTROLLER_APPROVAL_ENV: "operator-approved"}, + ) + + self.assertTrue(result["apply_gate"]["drain_gate_allow"]) + self.assertTrue(result["apply_gate"]["restart_class_authorized"]) + self.assertTrue(result["apply_authorized"], + "both authorizations pass; apply must be authorized") + + def test_denial_is_attributable_to_the_authorization_that_caused_it(self): + preview = self._call(role="author", restart_class="full_mcp_restart") + result = self._call( + role="author", + restart_class="full_mcp_restart", + dry_run=False, + drain_proof_json=self._clean_proof_for(preview), + ) + blob = " ".join(result["apply_gate"]["reasons"]).lower() + self.assertIn("restart class authorization denied", blob) + self.assertIn("full_mcp_restart", blob) + + +class TestProofCannotOverrideAuthorization(_RestartToolHarness): + """AC3 — a clean proof never overrides a class or requester-role denial.""" + + def test_clean_proof_cannot_override_role_denial(self): + for role in ("author", "reviewer", "merger", "reconciler"): + with self.subTest(role=role): + preview = self._call(role=role, restart_class="full_mcp_restart") + result = self._call( + role=role, + restart_class="full_mcp_restart", + dry_run=False, + drain_proof_json=self._clean_proof_for(preview), + ) + self.assertFalse(result["apply_authorized"]) + + def test_clean_proof_cannot_override_missing_controller_approval(self): + # Correct role, but the class demands controller approval and the + # environment carries none. + preview = self._call(role="operator", restart_class="full_mcp_restart") + self.assertFalse(preview["allow_restart"]) + result = self._call( + role="operator", + restart_class="full_mcp_restart", + dry_run=False, + drain_proof_json=self._clean_proof_for(preview), + ) + self.assertFalse(result["apply_authorized"]) + + def test_clean_proof_cannot_override_unknown_class(self): + preview = self._call(role="operator", restart_class="not_a_real_class", + env={CONTROLLER_APPROVAL_ENV: "yes"}) + self.assertFalse(preview["allow_restart"]) + result = self._call( + role="operator", + restart_class="not_a_real_class", + dry_run=False, + drain_proof_json=self._clean_proof_for(preview), + env={CONTROLLER_APPROVAL_ENV: "yes"}, + ) + self.assertFalse(result["apply_authorized"]) + + def test_clean_proof_cannot_override_missing_scope_target(self): + # worker_restart without target_session_id fails closed on scoping. + preview = self._call(role="operator", restart_class="worker_restart", + env={CONTROLLER_APPROVAL_ENV: "yes"}) + self.assertFalse(preview["allow_restart"]) + result = self._call( + role="operator", + restart_class="worker_restart", + dry_run=False, + drain_proof_json=self._clean_proof_for(preview), + env={CONTROLLER_APPROVAL_ENV: "yes"}, + ) + self.assertFalse(result["apply_authorized"]) + + +class TestBreakGlassDoesNotCollapseTheMatrix(_RestartToolHarness): + """AC4 — break-glass bypasses the drain proof only, never the class matrix.""" + + def test_break_glass_does_not_authorize_a_denied_class(self): + result = self._call( + role="author", + restart_class="host_restart", + dry_run=False, + request_break_glass=True, + env={BREAK_GLASS_ENV: "operator-issued"}, + ) + self.assertTrue(result["break_glass_authorized"]) + self.assertTrue(result["apply_gate"]["drain_gate_allow"], + "break-glass does satisfy the drain gate") + self.assertFalse(result["apply_gate"]["restart_class_authorized"]) + self.assertFalse(result["apply_authorized"], + "break-glass must not collapse the class matrix") + + def test_break_glass_across_every_worker_role_and_restricted_class(self): + for role in ("author", "reviewer", "merger", "reconciler"): + for klass in ("rolling_mcp_restart", "full_mcp_restart", + "host_restart"): + with self.subTest(role=role, restart_class=klass): + result = self._call( + role=role, + restart_class=klass, + dry_run=False, + request_break_glass=True, + env={BREAK_GLASS_ENV: "operator-issued"}, + ) + self.assertFalse(result["apply_authorized"]) + + def test_break_glass_still_works_when_the_class_is_authorized(self): + # Break-glass keeps its purpose: skipping the drain proof for a caller + # the matrix does allow. + result = self._call( + role="operator", + restart_class="full_mcp_restart", + dry_run=False, + request_break_glass=True, + env={BREAK_GLASS_ENV: "operator-issued", + CONTROLLER_APPROVAL_ENV: "operator-approved"}, + ) + self.assertTrue(result["apply_authorized"]) + self.assertEqual(result["apply_gate"]["verdict"], "break_glass") + + def test_break_glass_is_not_self_assertable(self): + # Requested but no environment authorization -> no bypass, and the + # unproven apply is denied. + result = self._call( + role="operator", + restart_class="full_mcp_restart", + dry_run=False, + request_break_glass=True, + env={CONTROLLER_APPROVAL_ENV: "operator-approved"}, + ) + self.assertTrue(result["break_glass_requested"]) + self.assertFalse(result["break_glass_authorized"]) + self.assertFalse(result["apply_authorized"]) + self.assertIn("incident", result) + + +class TestRestrictedClassesStayDenied(_RestartToolHarness): + """AC5 — restricted classes remain denied to unauthorized requesters.""" + + def test_restricted_classes_denied_for_worker_roles(self): + for role in ("author", "reviewer", "merger", "reconciler"): + for klass in ("rolling_mcp_restart", "full_mcp_restart", + "host_restart"): + with self.subTest(role=role, restart_class=klass): + preview = self._call( + role=role, + restart_class=klass, + env={CONTROLLER_APPROVAL_ENV: "yes"}, + ) + self.assertFalse(preview["allow_restart"]) + self.assertFalse(preview["permission_authorized"]) + self.assertFalse(preview["role_authorized"]) + + def test_host_restart_needs_controller_and_infrastructure_operator(self): + # controller approval alone is not enough for host_restart. + preview = self._call(role="controller", restart_class="host_restart", + env={CONTROLLER_APPROVAL_ENV: "yes"}) + self.assertFalse(preview["approval_satisfied"]) + self.assertFalse(preview["allow_restart"]) + + +class TestExistingPathsStillWork(_RestartToolHarness): + """AC6 — valid scoped and unscoped restart paths are unaffected.""" + + def test_dry_run_never_reports_apply_authorization(self): + result = self._call(role="operator", restart_class="full_mcp_restart", + env={CONTROLLER_APPROVAL_ENV: "yes"}) + self.assertNotIn("apply_authorized", result) + self.assertNotIn("apply_gate", result) + self.assertFalse(result["apply_supported"]) + self.assertFalse(result["restart_performed"]) + + def test_self_service_unscoped_classes_authorize_for_every_role(self): + for role in ("author", "reviewer", "merger", "reconciler", + "controller", "operator", "admin"): + for klass in ("client_reconnect", "session_reconnect"): + with self.subTest(role=role, restart_class=klass): + preview = self._call(role=role, restart_class=klass) + self.assertTrue(preview["allow_restart"]) + + def test_scoped_class_with_target_authorizes_and_applies(self): + env = {CONTROLLER_APPROVAL_ENV: "operator-approved"} + preview = self._call(role="operator", restart_class="worker_restart", + target_session_id="worker-1", env=env) + self.assertTrue(preview["allow_restart"]) + + result = self._call( + role="operator", + restart_class="worker_restart", + target_session_id="worker-1", + dry_run=False, + drain_proof_json=self._clean_proof_for(preview), + env=env, + ) + self.assertTrue(result["apply_authorized"]) + + def test_apply_still_denies_without_any_proof(self): + # The #661 hard gate is untouched by the conjunction. + result = self._call( + role="operator", + restart_class="full_mcp_restart", + dry_run=False, + env={CONTROLLER_APPROVAL_ENV: "operator-approved"}, + ) + self.assertFalse(result["apply_gate"]["drain_gate_allow"]) + self.assertTrue(result["apply_gate"]["restart_class_authorized"]) + self.assertFalse(result["apply_authorized"]) + self.assertEqual(result["incident"]["kind"], "restart_drain_gate_denied") + + def test_apply_denies_on_malformed_proof(self): + result = self._call( + role="operator", + restart_class="full_mcp_restart", + dry_run=False, + drain_proof_json="{not valid json", + env={CONTROLLER_APPROVAL_ENV: "operator-approved"}, + ) + self.assertFalse(result["apply_authorized"]) + self.assertTrue(any("invalid drain_proof_json" in reason + for reason in result["apply_gate"]["reasons"])) + + def test_tool_never_restarts_on_any_path(self): + for kwargs in ( + {"restart_class": "client_reconnect"}, + {"restart_class": "full_mcp_restart", "dry_run": False}, + {"restart_class": "host_restart", "dry_run": False, + "request_break_glass": True}, + ): + with self.subTest(**kwargs): + result = self._call(role="operator", env={ + CONTROLLER_APPROVAL_ENV: "yes", BREAK_GLASS_ENV: "yes"}, + **kwargs) + self.assertFalse(result["restart_performed"]) + self.assertFalse(result["apply_supported"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_mcp_restart_governance_docs.py b/tests/test_mcp_restart_governance_docs.py index 5b12e50..e49bcb4 100644 --- a/tests/test_mcp_restart_governance_docs.py +++ b/tests/test_mcp_restart_governance_docs.py @@ -105,3 +105,120 @@ def test_cross_links_do_not_embed_secrets(): text = _read(path) for marker in ("ghp_", "BEGIN PRIVATE KEY", "Authorization: Bearer"): assert marker not in text, f"{path} contains {marker!r}" + + +# --- Coordinator doc stays in lock-step with the tool (#886 review blocker B2) -- +# +# PR #882 moved the #661 drain-proof hard gate *into* gitea_request_mcp_restart, +# but the coordinator document still described the proof as "a separate child" +# and omitted both new parameters. Nothing referenced that document, so nothing +# caught the drift. These tests bind the prose to the real signature. + +COORDINATOR_DOC = REPO_ROOT / "docs" / "mcp-restart-coordinator.md" + +# Affirmative claims that were accurate before #661 landed and are now false. +# Matched against whitespace-normalized text so re-wrapping cannot hide them. +# Deliberately not the bare phrase "a separate child": the corrected prose uses +# it in a negation ("no longer a separate child operation"), and a guard that +# forbids naming the old behaviour would block explaining that it changed. +STALE_PRE_661_PHRASES = ( + "gated by a drain proof (a separate child)", + "is a later child gated by a drain proof", + "mutative apply path is explicitly out of scope", + "apply is gated by a drain proof (a separate child)", +) + + +def _documented_signature_block() -> str: + """The fenced signature block for the tool, as published in the doc.""" + text = _read(COORDINATOR_DOC) + marker = "gitea_request_mcp_restart(" + start = text.index(marker) + end = text.index("```", start) + return text[start:end] + + +def test_documented_signature_matches_the_real_tool_signature(): + import inspect + + import gitea_mcp_server + + block = _documented_signature_block() + real = inspect.signature(gitea_mcp_server.gitea_request_mcp_restart) + for name in real.parameters: + assert name in block, ( + f"docs/mcp-restart-coordinator.md documents no {name!r} parameter; " + "the published signature has drifted from the tool" + ) + + +def test_drain_proof_and_break_glass_parameters_are_documented(): + block = _documented_signature_block() + for name in ("drain_proof_json", "request_break_glass"): + assert name in block, f"signature block missing {name}" + + +def test_restart_class_and_target_scoping_parameters_survive(): + block = _documented_signature_block() + for name in ("restart_class", "target_session_id", "target_role", + "target_connector"): + assert name in block, f"signature block lost #663 parameter {name}" + + +def test_gate_is_documented_as_executing_inside_this_tool(): + lower = _read(COORDINATOR_DOC).lower() + assert "inside this tool" in lower, ( + "the coordinator doc must state that the drain-proof gate executes in " + "gitea_request_mcp_restart, not in a later child" + ) + assert "no longer a separate child operation" in lower + + +def test_stale_pre_661_wording_cannot_return(): + normalized = " ".join(_read(COORDINATOR_DOC).split()).lower() + for phrase in STALE_PRE_661_PHRASES: + assert phrase not in normalized, ( + f"stale pre-#661 wording returned to the coordinator doc: {phrase!r}" + ) + + +def test_dry_run_versus_apply_behavior_is_documented(): + lower = _read(COORDINATOR_DOC).lower() + assert "dry_run=true" in lower and "dry_run=false" in lower + assert "apply_supported" in lower and "restart_performed" in lower + assert "never restarts anything" in lower + + +def test_authorization_ordering_and_conjunction_are_documented(): + text = _read(COORDINATOR_DOC) + lower = text.lower() + assert "authorization ordering" in lower + assert "allow_restart" in text + assert "apply_authorized" in text + # The conjunction itself, and the attribution fields behind it. + assert "gate.allow and allow_restart" in text + for field in ("drain_gate_allow", "restart_class_authorized"): + assert field in text, f"doc omits apply_gate.{field}" + + +def test_break_glass_scope_is_documented_as_drain_proof_only(): + text = _read(COORDINATOR_DOC) + lower = text.lower() + assert "break-glass" in lower + assert "drain proof only" in lower, ( + "doc must state break-glass never bypasses the restart-class matrix" + ) + assert "GITEA_BREAKGLASS_RESTART_AUTHORIZATION" in text + + +def test_fail_closed_on_apply_is_documented(): + lower = _read(COORDINATOR_DOC).lower() + assert "fail closed" in lower + for condition in ("expired", "unclean", "tampered", "stale"): + assert condition in lower, f"fail-closed list omits {condition!r}" + + +def test_coordinator_doc_embeds_no_secrets(): + text = _read(COORDINATOR_DOC) + for marker in ("ghp_", "BEGIN PRIVATE KEY", "Authorization: Bearer"): + assert marker not in text, f"{COORDINATOR_DOC} contains {marker!r}" diff --git a/tests/test_restart_classes.py b/tests/test_restart_classes.py new file mode 100644 index 0000000..a8d627c --- /dev/null +++ b/tests/test_restart_classes.py @@ -0,0 +1,232 @@ +"""Permission, drain, routing, and audit matrix for restart classes (#663).""" + +from __future__ import annotations + +import os +from datetime import datetime, timezone + +import restart_coordinator as rc + +NOW = datetime(2026, 7, 24, 20, 0, tzinfo=timezone.utc) + + +def _inventory() -> dict: + return { + "inventory_complete": True, + "sessions": [ + { + "session_id": "requester", + "role": "author", + "profile": "prgs-author", + "pid": os.getpid(), + "status": "active", + "last_heartbeat_at": NOW.isoformat(), + }, + { + "session_id": "reviewer", + "role": "reviewer", + "profile": "prgs-reviewer", + "pid": os.getpid(), + "status": "active", + "last_heartbeat_at": NOW.isoformat(), + }, + ], + "leases": [ + { + "lease_id": "review-lease", + "session_id": "reviewer", + "role": "reviewer", + "phase": "reviewing", + "work_kind": "pr", + "work_number": 900, + "worktree_path": "/tmp/review-900", + "freshness": {"freshness": "active"}, + } + ], + } + + +def _evaluate( + restart_class: rc.RestartClass, + *, + role: str = "controller", + permissions: tuple[str, ...] | None = None, + approved: bool = True, + operator: bool = True, + **targets, +): + return rc.evaluate_restart_impact( + _inventory(), + now=NOW, + requesting_session_id="requester", + restart_class=restart_class, + requester_role=role, + requester_permissions=( + permissions if permissions is not None + else rc.permissions_for_role(role) + ), + controller_approved=approved, + operator_authorized=operator, + **targets, + ) + + +def test_policy_table_covers_exactly_all_nine_classes(): + assert set(rc.RESTART_CLASS_POLICIES) == set(rc.RestartClass) + assert len(rc.RESTART_CLASS_POLICIES) == 9 + for restart_class, policy in rc.RESTART_CLASS_POLICIES.items(): + assert policy.restart_class is restart_class + assert policy.required_permission + assert policy.expected_blast_radius in { + rc.BLAST_NONE, rc.BLAST_LOW, rc.BLAST_MEDIUM, rc.BLAST_HIGH + } + assert policy.drain_requirement + assert policy.approval_requirement + assert policy.audit_requirement + assert policy.recovery_behavior + + +def test_permission_matrix_allows_each_class_with_exact_permission(): + targets = { + rc.RestartClass.WORKER_RESTART: {"target_session_id": "reviewer"}, + rc.RestartClass.ROLE_RUNTIME_RESTART: {"target_role": "reviewer"}, + rc.RestartClass.CONNECTOR_RESTART: {"target_connector": "github"}, + } + for restart_class, policy in rc.RESTART_CLASS_POLICIES.items(): + report = _evaluate( + restart_class, + permissions=(policy.required_permission,), + **targets.get(restart_class, {}), + ) + assert report.permission_authorized, restart_class + assert report.role_authorized, restart_class + assert report.approval_satisfied, restart_class + assert report.audit_record["restart_class"] == restart_class.value + assert ( + report.audit_record["required_permission"] + == policy.required_permission + ) + + +def test_missing_or_nearby_permission_denies(): + report = _evaluate( + rc.RestartClass.ROLE_RUNTIME_RESTART, + permissions=("mcp.restart.worker.request",), + target_role="reviewer", + ) + assert report.verdict == rc.VERDICT_UNSAFE + assert not report.allow_restart + assert not report.permission_authorized + assert any("missing required permission" in r for r in report.reasons) + + +def test_unknown_restart_class_denies_fail_closed(): + report = rc.evaluate_restart_impact( + _inventory(), + now=NOW, + restart_class="surprise_reboot", + requester_role="admin", + requester_permissions=("mcp.restart.host.request",), + controller_approved=True, + operator_authorized=True, + ) + assert report.verdict == rc.VERDICT_UNSAFE + assert not report.allow_restart + assert report.restart_policy == {} + assert any("unknown restart class" in r for r in report.reasons) + + +def test_worker_roles_cannot_request_full_or_host_restart(): + for role in rc.WORKER_ROLES: + granted = rc.permissions_for_role(role) + assert "mcp.restart.full.request" not in granted + assert "mcp.restart.host.request" not in granted + report = _evaluate( + rc.RestartClass.FULL_MCP_RESTART, + role=role, + permissions=granted, + ) + assert not report.role_authorized + assert not report.allow_restart + + +def test_controller_approval_is_independent_of_permission(): + report = _evaluate( + rc.RestartClass.WORKER_RESTART, + approved=False, + target_session_id="reviewer", + ) + assert report.permission_authorized + assert not report.approval_satisfied + assert not report.allow_restart + + +def test_narrow_classes_do_not_inherit_full_drain_or_peer_lease_block(): + for restart_class in ( + rc.RestartClass.CLIENT_RECONNECT, + rc.RestartClass.SESSION_RECONNECT, + rc.RestartClass.CONFIGURATION_RELOAD, + ): + report = _evaluate(restart_class) + assert not report.restart_policy["full_drain_required"] + assert report.counts["leases_disruptive"] == 0 + assert report.counts["sessions_live_other"] == 0 + assert report.counts["critical_sections"] == 0 + assert report.counts["mutations"] == 0 + assert report.allow_restart, (restart_class, report.reasons) + + +def test_client_reconnect_does_not_wait_for_unrelated_terminal_lock(): + inventory = _inventory() + inventory["terminal_lock"] = {"terminal_pr": 901} + report = rc.evaluate_restart_impact( + inventory, + now=NOW, + requesting_session_id="requester", + restart_class=rc.RestartClass.CLIENT_RECONNECT, + requester_role="author", + requester_permissions=rc.permissions_for_role("author"), + ) + assert report.allow_restart + assert report.terminal_lock is None + + +def test_scoped_restart_only_counts_named_target(): + report = _evaluate( + rc.RestartClass.ROLE_RUNTIME_RESTART, + target_role="author", + ) + assert report.counts["leases_disruptive"] == 0 + assert report.affected_prs == [] + assert report.allow_restart + + reviewer = _evaluate( + rc.RestartClass.ROLE_RUNTIME_RESTART, + target_role="reviewer", + ) + assert reviewer.counts["leases_disruptive"] == 1 + assert reviewer.affected_prs == [900] + assert not reviewer.allow_restart + + +def test_missing_scoped_target_denies_instead_of_widening(): + for restart_class in ( + rc.RestartClass.WORKER_RESTART, + rc.RestartClass.ROLE_RUNTIME_RESTART, + rc.RestartClass.CONNECTOR_RESTART, + ): + report = _evaluate(restart_class) + assert not report.allow_restart + assert any("target required" in r for r in report.reasons) + + +def test_only_full_and_host_classes_require_full_drain(): + requiring_full = { + restart_class + for restart_class, policy in rc.RESTART_CLASS_POLICIES.items() + if policy.full_drain_required + } + assert requiring_full == { + rc.RestartClass.FULL_MCP_RESTART, + rc.RestartClass.HOST_RESTART, + } diff --git a/tests/test_webui_sanctioned_restart.py b/tests/test_webui_sanctioned_restart.py index a495935..d794a75 100644 --- a/tests/test_webui_sanctioned_restart.py +++ b/tests/test_webui_sanctioned_restart.py @@ -444,6 +444,12 @@ class TestAuditEmission(unittest.TestCase): ) self.assertEqual(record["target"]["namespace"], NAMESPACE) self.assertEqual(record["target"]["mode"], "restart") + self.assertEqual( + record["target"]["restart_class"], "role_runtime_restart" + ) + self.assertEqual( + record["metadata"]["restart_class"], "role_runtime_restart" + ) self.assertEqual(record["result"], console_audit.RESULT_ALLOWED) self.assertEqual(record["actor"]["subject"], "admin@example.test") self.assertFalse(record["metadata"]["process_kill_executed"]) diff --git a/tests/test_webui_sessions_view.py b/tests/test_webui_sessions_view.py new file mode 100644 index 0000000..4799b77 --- /dev/null +++ b/tests/test_webui_sessions_view.py @@ -0,0 +1,739 @@ +"""Tests for the Runtime and session view (Phase 1, #641). + +Covers clean and stale session rendering, contamination marker surfacing, +worktree binding display, sanctioned recovery links (no pkill), nav/live +status, and the JSON API export. + +Also pins the two invariants a reviewer found violated at head a81db754: +degraded ownership sections must render as *unknown* rather than as an +affirmative "none"/"unbound", and contamination payload text must be redacted +at the display boundary rather than trusted from the write-time denylist. +""" + +from __future__ import annotations + +import os +import sys +import unittest +from pathlib import Path +from unittest import mock + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from tests.webui_testclient import TestClient + +from webui.app import create_app +from webui.inventory import ( + AUTHORITY_CONTROL_PLANE_DB, + AUTHORITY_FILESYSTEM, + InventorySection, + InventorySnapshot, + STATUS_DEGRADED, + STATUS_OK, + STATUS_UNAVAILABLE, +) +from webui.nav import NAV_GROUPS, STUB_PAGES, iter_nav_items +from webui.runtime_health import FileHash, RuntimeSnapshot +from webui.session_loader import ( + ContaminationMarker, + SessionRow, + SessionViewSnapshot, + _build_session_rows, + _inspect_contamination, + load_session_view_snapshot, + snapshot_to_dict, +) +from webui.session_views import render_sessions_page + + +def _runtime( + *, + stale: str | None = None, + profile: str = "prgs-author", + role: str = "author", +) -> RuntimeSnapshot: + return RuntimeSnapshot( + project_id="gitea-tools", + repo_root="/tmp/repo", + remote="prgs", + host="gitea.prgs.cc", + profile_name=profile, + role_kind=role, + config_model="v2-contexts", + profile_mode="dynamic-profile", + profile_source="config file profile", + authenticated_username="jcwalker3", + identity_error=None, + repo_sha="a" * 40, + remote_master_sha="a" * 40, + commits_behind_master=0, + stale_runtime_warning=stale, + shell_health={"shell_use_allowed": True, "consecutive_spawn_failures": 0}, + workflow_hashes=( + FileHash(label="SKILL.md", path="skills/llm-project-workflow/SKILL.md", sha256="abc"), + ), + schema_hashes=(), + restart_guidance="docs/mcp-namespace-eof-recovery.md", + fetch_error=None, + ) + + +def _inventory( + *, + sessions: tuple[dict, ...] = (), + leases: tuple[dict, ...] = (), + locks: tuple[dict, ...] = (), + worktrees: tuple[dict, ...] = (), + namespaces: tuple[dict, ...] = (), + statuses: dict[str, str] | None = None, +) -> InventorySnapshot: + """Build a snapshot; ``statuses`` degrades named sections (default all ok).""" + status_of = statuses or {} + + def _status(name: str) -> str: + return status_of.get(name, STATUS_OK) + + sections = ( + InventorySection( + name="sessions", + authority=AUTHORITY_CONTROL_PLANE_DB, + status=_status("sessions"), + items=sessions, + ), + InventorySection( + name="leases", + authority=AUTHORITY_CONTROL_PLANE_DB, + status=_status("leases"), + items=leases, + ), + InventorySection( + name="locks", + authority=AUTHORITY_FILESYSTEM, + status=_status("locks"), + items=locks, + ), + InventorySection( + name="worktrees", + authority=AUTHORITY_FILESYSTEM, + status=_status("worktrees"), + items=worktrees, + ), + InventorySection( + name="namespaces", + authority=AUTHORITY_FILESYSTEM, + status=_status("namespaces"), + items=namespaces + or ( + { + "profile_name": "prgs-author", + "role": "author", + "mcp_namespace": "gitea-author", + "capability_summary": { + "can_author": True, + "can_review": False, + "can_merge": False, + }, + "active": True, + }, + ), + reason="only the profile serving this web process is observable", + ), + ) + index = {section.name: section for section in sections} + return InventorySnapshot( + generated_at="2026-07-25T00:00:00+00:00", + sections=sections, + collisions=(), + correlations=(), + scan_ms=1.0, + _section_index=index, + ) + + +def _clean_session() -> dict: + return { + "session_id": "prgs-author-111-clean", + "role": "author", + "profile": "prgs-author", + "namespace": "gitea-author", + "pid": 1111, + "pid_alive": True, + "status": "active", + "started_at": "2026-07-25T00:00:00Z", + "last_heartbeat_at": "2026-07-25T01:00:00Z", + } + + +def _stale_session() -> dict: + return { + "session_id": "prgs-author-222-stale", + "role": "author", + "profile": "prgs-author", + "namespace": "gitea-author", + "pid": 2222, + "pid_alive": False, + "status": "active", + "started_at": "2026-07-24T00:00:00Z", + "last_heartbeat_at": "2026-07-24T01:00:00Z", + } + + +class TestBuildSessionRows(unittest.TestCase): + def test_clean_session_has_no_stale_or_contamination_flags(self): + inventory = _inventory( + sessions=(_clean_session(),), + leases=( + { + "lease_id": "lease-clean", + "session_id": "prgs-author-111-clean", + "status": "active", + "expired": False, + "work_kind": "issue", + "work_number": 641, + }, + ), + locks=( + { + "issue_number": 641, + "branch_name": "feat/issue-641-runtime-session-view", + "worktree_path": "~/Development/Gitea-Tools/branches/feat-issue-641", + "live": True, + }, + ), + ) + rows = _build_session_rows(inventory, contamination=()) + self.assertEqual(len(rows), 1) + row = rows[0] + self.assertEqual(row.session_id, "prgs-author-111-clean") + self.assertEqual(row.role, "author") + self.assertEqual(row.namespace, "gitea-author") + self.assertEqual(row.pid_alive, True) + self.assertEqual(row.lease_ids, ("lease-clean",)) + self.assertEqual(row.work_refs, ("issue#641",)) + self.assertTrue(row.worktree_paths) + self.assertEqual(row.stale_flags, ()) + self.assertEqual(row.contamination_flags, ()) + + def test_stale_session_flags_dead_pid(self): + inventory = _inventory(sessions=(_stale_session(),)) + rows = _build_session_rows(inventory, contamination=()) + self.assertEqual(rows[0].stale_flags, ("pid-dead",)) + + def test_contamination_marker_binds_to_session(self): + inventory = _inventory(sessions=(_clean_session(),)) + marker = ContaminationMarker( + kind="runtime_recovery_contamination", + on_disk=True, + has_payload=True, + summary="manual daemon kill", + reason_class="manual_daemon_kill", + session_id="prgs-author-111-clean", + role="author", + command_summary="pkill -f mcp_server.py", + cleared=False, + ) + rows = _build_session_rows(inventory, contamination=(marker,)) + self.assertIn("runtime_recovery_contamination", rows[0].contamination_flags) + + def test_process_wide_contamination_surfaces_on_all_sessions(self): + inventory = _inventory(sessions=(_clean_session(), _stale_session())) + marker = ContaminationMarker( + kind="stable_branch_contamination", + on_disk=True, + has_payload=True, + summary="direct master push attempt", + reason_class="stable_branch_push", + session_id=None, + cleared=False, + ) + rows = _build_session_rows(inventory, contamination=(marker,)) + self.assertEqual(len(rows), 2) + for row in rows: + self.assertTrue( + any("stable_branch_contamination" in f for f in row.contamination_flags) + ) + + +class TestRenderSessionsPage(unittest.TestCase): + def _snapshot( + self, + *, + sessions: tuple[dict, ...], + contamination: tuple[ContaminationMarker, ...] = (), + stale_runtime: str | None = None, + ) -> SessionViewSnapshot: + inventory = _inventory( + sessions=sessions, + leases=( + { + "lease_id": "lease-1", + "session_id": sessions[0]["session_id"] if sessions else "", + "status": "active", + "expired": False, + "work_kind": "issue", + "work_number": 641, + }, + ) + if sessions + else (), + locks=( + { + "issue_number": 641, + "worktree_path": "branches/feat-issue-641", + }, + ) + if sessions + else (), + worktrees=( + { + "rel_path": "branches/feat-issue-641", + "branch": "feat/issue-641-runtime-session-view", + "classification": "active_issue_work", + "registered_worktree": True, + "dirty": False, + }, + ), + ) + rows = _build_session_rows(inventory, contamination) + return SessionViewSnapshot( + runtime=_runtime(stale=stale_runtime), + inventory=inventory, + sessions=rows, + contamination_markers=contamination, + ) + + def test_clean_session_render(self): + html = render_sessions_page(self._snapshot(sessions=(_clean_session(),))) + self.assertIn("Runtime and sessions", html) + self.assertIn("prgs-author-111-clean", html) + self.assertIn("gitea-author", html) + self.assertIn("branches/feat-issue-641", html) + self.assertIn("Sanctioned recovery", html) + self.assertIn("docs/mcp-namespace-eof-recovery.md", html) + # Recovery section must name reconnect and forbid manual kill. + recovery_idx = html.lower().find("sanctioned recovery") + self.assertGreaterEqual(recovery_idx, 0) + recovery = html[recovery_idx:].lower() + self.assertIn("reconnect", recovery) + self.assertIn("contamination", recovery) + self.assertIn("not recovery", recovery) + self.assertNotIn("run pkill", recovery) + self.assertNotIn("killall", recovery) + + def test_stale_session_render(self): + html = render_sessions_page(self._snapshot(sessions=(_stale_session(),))) + self.assertIn("prgs-author-222-stale", html) + self.assertIn("pid-dead", html) + self.assertIn("badge-stale", html) + + def test_contamination_render_is_not_silent(self): + marker = ContaminationMarker( + kind="runtime_recovery_contamination", + on_disk=True, + has_payload=True, + summary="manual kill", + reason_class="manual_daemon_kill", + session_id="prgs-author-111-clean", + command_summary="pkill -f mcp_server.py", + cleared=False, + ) + html = render_sessions_page( + self._snapshot(sessions=(_clean_session(),), contamination=(marker,)) + ) + self.assertIn("Contamination markers", html) + self.assertIn("runtime_recovery_contamination", html) + self.assertIn("ACTIVE", html) + self.assertIn("badge-blocked", html) + + def test_stale_runtime_banner(self): + html = render_sessions_page( + self._snapshot( + sessions=(_clean_session(),), + stale_runtime="server behind master by 3 commits", + ) + ) + self.assertIn("Stale runtime", html) + self.assertIn("server behind master", html) + + +class TestSessionLoaderComposition(unittest.TestCase): + def test_load_with_injected_sources(self): + inventory = _inventory(sessions=(_clean_session(), _stale_session())) + snap = load_session_view_snapshot( + load_runtime=lambda: _runtime(), + load_inventory=lambda: inventory, + inspect_contamination=lambda **_k: { + "on_disk": False, + "has_payload": False, + "summary": "absent", + }, + load_contamination_payload=lambda **_k: None, + ) + self.assertEqual(len(snap.sessions), 2) + self.assertEqual(snap.stale_session_count, 1) + self.assertEqual(snap.contaminated_session_count, 0) + data = snapshot_to_dict(snap) + self.assertEqual(data["view"], "runtime-sessions") + self.assertEqual(data["issue"], 641) + self.assertTrue(data["read_only"]) + self.assertEqual(data["session_counts"]["total"], 2) + self.assertEqual(data["session_counts"]["stale"], 1) + self.assertIn("recovery_docs", data) + + +class TestSessionsRoutes(unittest.TestCase): + def setUp(self): + self.client = TestClient(create_app()) + inventory = _inventory( + sessions=(_clean_session(), _stale_session()), + leases=( + { + "lease_id": "lease-x", + "session_id": "prgs-author-111-clean", + "status": "active", + "expired": False, + "work_kind": "issue", + "work_number": 641, + }, + ), + locks=( + { + "issue_number": 641, + "worktree_path": "branches/feat-issue-641", + }, + ), + worktrees=( + { + "rel_path": "branches/feat-issue-641", + "branch": "feat/issue-641-runtime-session-view", + "classification": "active_issue_work", + "registered_worktree": True, + "dirty": False, + }, + ), + ) + rows = _build_session_rows(inventory, contamination=()) + self.snapshot = SessionViewSnapshot( + runtime=_runtime(stale="stale for test"), + inventory=inventory, + sessions=rows, + contamination_markers=(), + ) + self._patch = mock.patch( + "webui.app.load_session_view_snapshot", + return_value=self.snapshot, + ) + self._patch.start() + + def tearDown(self): + self._patch.stop() + + def test_sessions_page_live(self): + response = self.client.get("/sessions") + self.assertEqual(response.status_code, 200) + self.assertIn("Runtime and sessions", response.text) + self.assertIn("prgs-author-111-clean", response.text) + self.assertIn("prgs-author-222-stale", response.text) + self.assertIn("pid-dead", response.text) + self.assertIn("Sanctioned recovery", response.text) + self.assertNotIn("Phase 1 shell placeholder", response.text) + self.assertNotIn("child issue of #425", response.text.lower()) + + def test_api_sessions_json(self): + for path in ("/api/sessions", "/api/v1/sessions"): + response = self.client.get(path) + self.assertEqual(response.status_code, 200, path) + data = response.json() + self.assertEqual(data["view"], "runtime-sessions") + self.assertEqual(data["session_counts"]["total"], 2) + self.assertEqual(data["session_counts"]["stale"], 1) + self.assertTrue(data["read_only"]) + self.assertEqual(data["mutations"], []) + + def test_nav_marks_sessions_live(self): + sessions_items = [ + item for item in iter_nav_items() if item.href == "/sessions" + ] + self.assertEqual(len(sessions_items), 1) + self.assertEqual(sessions_items[0].status, "live") + self.assertNotIn("/sessions", STUB_PAGES) + # Home page should not mark Sessions as stub. + home = self.client.get("/") + self.assertEqual(home.status_code, 200) + self.assertIn('href="/sessions"', home.text) + # Stub marker only appears next to remaining stub destinations. + self.assertNotIn( + 'href="/sessions">Sessions (stub)', + home.text, + ) + + +class TestDegradedOwnershipAuthority(unittest.TestCase): + """B1: a section that could not be read must never render as absence.""" + + def _snapshot(self, inventory: InventorySnapshot) -> SessionViewSnapshot: + return SessionViewSnapshot( + runtime=_runtime(), + inventory=inventory, + sessions=_build_session_rows(inventory, contamination=()), + contamination_markers=(), + ) + + def test_unavailable_leases_mark_row_authority_unproven(self): + inventory = _inventory( + sessions=(_clean_session(),), + statuses={"leases": STATUS_UNAVAILABLE}, + ) + row = _build_session_rows(inventory, contamination=())[0] + self.assertEqual(row.lease_ids, ()) + self.assertEqual(row.lease_authority, STATUS_UNAVAILABLE) + # Worktree binding is correlated through lease work numbers, so it + # inherits the unreadable lease section. + self.assertEqual(row.worktree_authority, STATUS_UNAVAILABLE) + self.assertFalse(row.ownership_authority_complete) + + def test_readable_locks_are_not_reported_unbound_when_leases_degrade(self): + # The narrow variant: locks hold a real worktree_path and read cleanly, + # but the lease section that supplies the correlating work number does + # not. The row must say unknown, not "unbound". + inventory = _inventory( + sessions=(_clean_session(),), + locks=( + { + "issue_number": 641, + "worktree_path": "branches/feat-issue-641", + }, + ), + statuses={"leases": STATUS_DEGRADED}, + ) + row = _build_session_rows(inventory, contamination=())[0] + self.assertEqual(row.worktree_paths, ()) + self.assertEqual(row.worktree_authority, STATUS_DEGRADED) + self.assertFalse(row.ownership_authority_complete) + + def test_degraded_render_says_unknown_not_none_or_unbound(self): + inventory = _inventory( + sessions=(_clean_session(),), + statuses={"leases": STATUS_UNAVAILABLE, "locks": STATUS_UNAVAILABLE}, + ) + html = render_sessions_page(self._snapshot(inventory)) + self.assertIn("unknown (inventory unavailable)", html) + self.assertIn("authority unproven", html) + self.assertIn("Ownership authority incomplete", html) + # The affirmative-absence strings must be gone from the row entirely. + self.assertNotIn(">none<", html) + self.assertNotIn(">unbound<", html) + + def test_clean_inventory_still_renders_affirmative_absence(self): + # Guards against over-correcting B1 into "everything is unknown". + inventory = _inventory(sessions=(_clean_session(),)) + html = render_sessions_page(self._snapshot(inventory)) + self.assertIn(">none<", html) + self.assertIn(">unbound<", html) + # The column legend mentions "unknown (inventory …)" as static copy, so + # assert on the per-row marker and the concrete statuses instead. + self.assertNotIn("authority unproven", html) + self.assertNotIn("unknown (inventory unavailable)", html) + self.assertNotIn("unknown (inventory degraded)", html) + self.assertNotIn("Ownership authority incomplete", html) + + def test_json_export_carries_snapshot_and_per_row_authority(self): + inventory = _inventory( + sessions=(_clean_session(),), + statuses={"locks": STATUS_UNAVAILABLE}, + ) + data = snapshot_to_dict(self._snapshot(inventory)) + self.assertFalse(data["ownership_authority_complete"]) + self.assertEqual( + data["ownership_section_status"]["locks"], STATUS_UNAVAILABLE + ) + self.assertEqual(data["ownership_section_status"]["leases"], STATUS_OK) + self.assertIn("unknown, not unowned", data["ownership_note"]) + + row = data["sessions"][0] + self.assertTrue(row["lease_authority_complete"]) + self.assertFalse(row["worktree_authority_complete"]) + self.assertEqual(row["worktree_authority"], STATUS_UNAVAILABLE) + self.assertFalse(row["ownership_authority_complete"]) + self.assertIn("unknown, not unowned", row["ownership_note"]) + + def test_json_export_is_affirmative_when_every_source_reads(self): + inventory = _inventory(sessions=(_clean_session(),)) + data = snapshot_to_dict(self._snapshot(inventory)) + self.assertTrue(data["ownership_authority_complete"]) + self.assertTrue(data["sessions"][0]["ownership_authority_complete"]) + + def test_missing_session_list_is_not_reported_as_no_sessions(self): + inventory = _inventory(statuses={"sessions": STATUS_UNAVAILABLE}) + html = render_sessions_page(self._snapshot(inventory)) + self.assertIn("could not be read", html) + self.assertIn("not evidence that no sessions exist", html) + + def test_expired_lease_flags_row_as_stale(self): + inventory = _inventory( + sessions=(_clean_session(),), + leases=( + { + "lease_id": "lease-expired-1", + "session_id": "prgs-author-111-clean", + "status": "active", + "expired": True, + "work_kind": "issue", + "work_number": 641, + }, + ), + ) + row = _build_session_rows(inventory, contamination=())[0] + self.assertIn("lease-expired", row.stale_flags) + self.assertIn("active-lease-past-expiry", row.stale_flags) + + +class TestContaminationRedaction(unittest.TestCase): + """B2: marker payload text is redacted at the display boundary.""" + + def _marker(self, payload: dict) -> ContaminationMarker: + return _inspect_contamination( + "runtime_recovery_contamination", + remote="prgs", + inspect=lambda **_k: { + "on_disk": True, + "has_payload": True, + "summary": "", + }, + load=lambda **_k: payload, + ) + + def test_home_paths_are_collapsed(self): + home = os.path.expanduser("~") + marker = self._marker( + {"command_summary": f"pkill -f {home}/Development/Gitea-Tools/x.py"} + ) + self.assertNotIn(home, marker.command_summary) + self.assertIn("~/Development/Gitea-Tools/x.py", marker.command_summary) + + def test_secrets_missed_by_the_write_time_denylist_are_redacted(self): + # Each of these was verified in review to survive + # stable_branch_push_guard.redact_command untouched. + cases = ( + ("curl -H 'X-Api-Key: SUPERSECRET123' https://example.invalid", "SUPERSECRET123"), + ("cmd --password hunter2 origin master", "hunter2"), + ("PRIVATE_KEY=abc123 python deploy.py", "abc123"), + ("fetch https://user:pw@example.invalid/x.git", "user:pw"), + ) + for raw, secret in cases: + with self.subTest(raw=raw): + marker = self._marker({"command_summary": raw}) + self.assertNotIn(secret, marker.command_summary) + self.assertIn("[redacted]", marker.command_summary) + + def test_command_summary_is_redacted_not_removed(self): + # It is legitimate #630 evidence: the operator must still see which + # daemon was killed. + marker = self._marker( + { + "command_summary": "pkill -f gitea_mcp_server.py", + "reason_class": "manual_daemon_kill", + "session_id": "prgs-author-111-clean", + "role": "author", + } + ) + self.assertIn("pkill -f gitea_mcp_server.py", marker.command_summary) + self.assertEqual(marker.reason_class, "manual_daemon_kill") + self.assertEqual(marker.session_id, "prgs-author-111-clean") + self.assertEqual(marker.role, "author") + + def test_rendered_page_exposes_no_home_path_from_a_marker(self): + home = os.path.expanduser("~") + marker = self._marker( + { + "command_summary": f"pkill -f {home}/Development/Gitea-Tools/x.py", + "reason_class": "manual_daemon_kill", + } + ) + inventory = _inventory(sessions=(_clean_session(),)) + html = render_sessions_page( + SessionViewSnapshot( + runtime=_runtime(), + inventory=inventory, + sessions=_build_session_rows(inventory, (marker,)), + contamination_markers=(marker,), + ) + ) + self.assertIn("Contamination markers", html) + self.assertNotIn(home, html) + + +class TestSessionsPageEscaping(unittest.TestCase): + """Hostile values from every rendered source stay inert (N2).""" + + HOSTILE = '' + + def test_hostile_session_and_marker_values_are_escaped(self): + session = dict(_clean_session()) + session["session_id"] = f"sid-{self.HOSTILE}" + session["role"] = self.HOSTILE + session["profile"] = self.HOSTILE + session["namespace"] = self.HOSTILE + session["status"] = self.HOSTILE + inventory = _inventory( + sessions=(session,), + leases=( + { + "lease_id": self.HOSTILE, + "session_id": session["session_id"], + "status": "active", + "expired": False, + "work_kind": self.HOSTILE, + "work_number": 641, + }, + ), + locks=( + { + "issue_number": 641, + "worktree_path": self.HOSTILE, + }, + ), + ) + marker = ContaminationMarker( + kind="runtime_recovery_contamination", + on_disk=True, + has_payload=True, + summary=self.HOSTILE, + reason_class=self.HOSTILE, + session_id=session["session_id"], + role=self.HOSTILE, + command_summary=self.HOSTILE, + cleared=False, + ) + html = render_sessions_page( + SessionViewSnapshot( + runtime=_runtime(), + inventory=inventory, + sessions=_build_session_rows(inventory, (marker,)), + contamination_markers=(marker,), + ) + ) + self.assertNotIn("