Compare commits

...
Author SHA1 Message Date
sysadminandClaude Opus 5 220361ad94 fix(restart): require both authorizations for apply, correct coordinator doc
Addresses the two blockers raised in the PR #886 review (comment 16559) for
issue #663.

B1 — apply_authorized ignored restart-class authorization.

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 this branch merged it. The union preserved both, but the apply
decision consulted only the drain gate:

    payload["apply_authorized"] = gate.allow

so a clean drain proof — or an authorized break-glass, which needs no proof
at all — reported apply_authorized: True for a class the least-privilege
matrix had just denied, in the same payload carrying allow_restart: False
and "role 'author' may not request full_mcp_restart". One environment
variable therefore collapsed the whole nine-class matrix for the apply
decision, including host_restart.

The apply decision is now the conjunction of both authorizations, and
apply_gate carries drain_gate_allow and restart_class_authorized so a denial
is attributable to the authorization that produced it. Break-glass keeps its
purpose — bypassing the drain proof — and never bypasses the class matrix.
No existing fail-closed behaviour is weakened: allow_restart, drain-proof
verification, fingerprint binding, and requester authorization are untouched.

B2 — docs/mcp-restart-coordinator.md described pre-#661 behaviour.

The document still called the drain proof "a separate child" and omitted
drain_proof_json and request_break_glass from the published signature, so a
safety document asserted there was no gate where a gate now exists. It now
documents both parameters, states that the gate executes inside this tool,
and records dry-run versus apply behaviour, authorization ordering, the
break-glass scope, and fail-closed conditions as implemented.

Regression coverage.

tests/test_issue_886_apply_authorization_conjunction.py exercises the MCP
tool itself, which previously had no test at all — that absence is why the
defect shipped. It pins both conjunction directions, proves a clean proof
cannot override a role, approval, unknown-class, or missing-target denial,
proves break-glass does not collapse the matrix for any worker role or
restricted class, and proves the existing scoped and unscoped paths and the
#661 denials still hold. Against the pre-fix tree 24 of these fail; against
this commit all 19 pass with 45 subtests.

tests/test_mcp_restart_governance_docs.py now binds the published signature
to inspect.signature() of the real tool and forbids the stale pre-#661
phrasing, so the drift that produced B2 cannot return unnoticed.

Verification: targeted restart/drain/governance/webui suites 194 passed,
113 subtests. Full suite 23 failed, 5230 passed, 6 skipped, 912 subtests —
the failure set is identical to the reviewed baseline at 9bc021e
(23 failed, 5201 passed), with +29 passing from the added tests and no new
or changed failure. Zero conflict markers; py_compile passes; the #882
union remains intact in both directions.

Refs #663, PR #886

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01V6xFqovhbArPv61j9KCGkL
2026-07-25 02:36:41 -04:00
sysadminandClaude Opus 4.8 9bc021e9c0 Merge master into feat/issue-663-restart-classes (resolve #886 conflict)
Brings PR #886 up to date with master @ 2f4dec8323
(8 commits behind), resolving the single conflicted file.

Conflict: gitea_mcp_server.py, both hunks inside gitea_request_mcp_restart.
Both sides were purely additive to the same tool, so both are kept in full:

- Branch side (#663, restart classes): parameters restart_class,
  target_session_id, target_role, target_connector; payload keys
  controller_approval_authorized, requester_role, requester_permissions.
- Master side (#661 via PR #882, drain-proof hard gate): parameters
  drain_proof_json, request_break_glass; the explanatory comment describing
  the apply-path hard gate and break-glass authorization.

No behaviour from either side was dropped, reordered, or reimplemented. Every
parameter from both sides is already consumed by the auto-merged function body
(restart_class and the three target_* arguments flow into the coordinator call;
drain_proof_json and request_break_glass drive the dry_run=False hard gate), so
the union is the only resolution that keeps the merged function coherent.

Validation on the merged tree:

  python -m pytest tests/test_drain_proof.py tests/test_restart_classes.py \
    tests/test_restart_coordinator.py tests/test_mcp_restart_paths.py \
    tests/test_mcp_restart_governance_docs.py tests/test_webui_sanctioned_restart.py \
    tests/test_issue_662_post_restart_reconcile.py -q
  # 165 passed, 68 subtests passed

py_compile on gitea_mcp_server.py passes and no conflict markers remain.

Closes #663

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2026-07-25 01:16:09 -04:00
jcwalker3 930dc24632 Merge branch 'master' into feat/issue-663-restart-classes 2026-07-24 22:34:33 -05:00
jcwalker3 41622c5985 Merge branch 'master' into feat/issue-663-restart-classes 2026-07-24 21:27:15 -05:00
jcwalker3 301c78de20 Merge branch 'master' into feat/issue-663-restart-classes 2026-07-24 21:06:21 -05:00
sysadmin 714190e02a feat: enforce MCP restart class permissions (#663) 2026-07-24 18:10:22 -04:00
9 changed files with 1302 additions and 28 deletions
+53
View File
@@ -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).
+67 -10
View File
@@ -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 disrupted. Uncoordinated restarts destroy in-flight author/reviewer/merger work
and give operators no way to see what they are about to break. 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 sanctioned entry point for restart evaluation post-#657 (which inventoried the
restart/reload/kill paths). The **mutative apply** path — actually performing a restart/reload/kill paths).
restart — is a later child gated by a drain proof and is explicitly out of
scope here. 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 ## 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. | | `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). | | `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). | | `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 ## Dimensions evaluated
@@ -77,17 +90,61 @@ authorization is present.
```text ```text
gitea_request_mcp_restart(remote, host, org, repo, gitea_request_mcp_restart(remote, host, org, repo,
dry_run=True, request_override=False, 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 It **never restarts anything**: `apply_supported` is always `false` and
always `false`; passing `dry_run=False` performs no restart and reports that `restart_performed` is always `false`.
apply is gated by a drain proof (a separate child).
### 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 ## Audit
Every evaluation carries an `audit_record` (event, coordinator version, verdict, 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 auditable. No secrets flow through the coordinator — session ids, pids, and
profiles are operational metadata only. profiles are operational metadata only.
+57 -3
View File
@@ -22343,12 +22343,17 @@ def gitea_request_mcp_restart(
request_override: bool = False, request_override: bool = False,
session_id: str | None = None, session_id: str | None = None,
limit: int = 200, 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, drain_proof_json: str | None = None,
request_break_glass: bool = False, request_break_glass: bool = False,
) -> dict: ) -> dict:
"""Evaluate a proposed MCP restart and return an impact preview (#658). """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 leases/locks, in-flight issue/PR work, mutations, worktrees) and returns a
blast-radius impact report with a ``safe`` / ``unsafe`` / ``override`` blast-radius impact report with a ``safe`` / ``unsafe`` / ``override``
verdict, so the console (#642/#652) and operators can see what a restart verdict, so the console (#642/#652) and operators can see what a restart
@@ -22362,7 +22367,16 @@ def gitea_request_mcp_restart(
only when ``request_break_glass`` is set *and* the environment carries only when ``request_break_glass`` is set *and* the environment carries
``GITEA_BREAKGLASS_RESTART_AUTHORIZATION``. Even an authorized gate performs ``GITEA_BREAKGLASS_RESTART_AUTHORIZATION``. Even an authorized gate performs
no restart here; actual execution is a further child. The gate outcome is 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 Operator override authority is read from the process environment
(``GITEA_OPERATOR_RESTART_OVERRIDE_AUTHORIZATION``), never self-asserted by (``GITEA_OPERATOR_RESTART_OVERRIDE_AUTHORIZATION``), never self-asserted by
@@ -22447,6 +22461,9 @@ def gitea_request_mcp_restart(
profile = get_profile() profile = get_profile()
profile_name = (profile.get("profile_name") or "").strip() or "session" 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()}" sid = (session_id or "").strip() or f"{profile_name}-{os.getpid()}"
# Override authority is read from the environment only — a worker session # Override authority is read from the environment only — a worker session
@@ -22456,6 +22473,15 @@ def gitea_request_mcp_restart(
(os.environ.get("GITEA_OPERATOR_RESTART_OVERRIDE_AUTHORIZATION") or "").strip() (os.environ.get("GITEA_OPERATOR_RESTART_OVERRIDE_AUTHORIZATION") or "").strip()
) )
operator_override = bool(request_override and operator_authorized) 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 = { inventory = {
"sessions": sessions, "sessions": sessions,
@@ -22470,6 +22496,14 @@ def gitea_request_mcp_restart(
operator_override=operator_override, operator_override=operator_override,
requesting_session_id=sid, requesting_session_id=sid,
dry_run=True, # coordinator is always analysis-only (#658) 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() payload = report.as_dict()
@@ -22481,6 +22515,9 @@ def gitea_request_mcp_restart(
payload["requesting_session_id"] = sid payload["requesting_session_id"] = sid
payload["operator_override_requested"] = bool(request_override) payload["operator_override_requested"] = bool(request_override)
payload["operator_override_authorized"] = operator_authorized 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 # 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 # 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 # False) must present a valid, unexpired, clean drain proof, or it is denied
@@ -22518,8 +22555,25 @@ def gitea_request_mcp_restart(
gate_payload["reasons"] = [proof_parse_error] + list( gate_payload["reasons"] = [proof_parse_error] + list(
gate_payload.get("reasons") or [] 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_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_requested"] = bool(request_break_glass)
payload["break_glass_authorized"] = break_glass_authorized payload["break_glass_authorized"] = break_glass_authorized
# Even an authorized gate performs no restart here: execution is a later # Even an authorized gate performs no restart here: execution is a later
+370 -14
View File
@@ -28,11 +28,12 @@ from __future__ import annotations
from dataclasses import dataclass, field from dataclasses import dataclass, field
from datetime import datetime, timezone from datetime import datetime, timezone
from enum import Enum
from typing import Any, Mapping, Sequence from typing import Any, Mapping, Sequence
import lease_lifecycle 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. # Restart verdicts. Exactly the three the acceptance criteria name.
VERDICT_SAFE = "safe" VERDICT_SAFE = "safe"
@@ -54,6 +55,194 @@ LEASE_FRESHNESS_LIVE = "active"
DEFAULT_SESSION_HEARTBEAT_STALE_SECONDS = 900 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: def _utc_now() -> datetime:
return datetime.now(timezone.utc) return datetime.now(timezone.utc)
@@ -75,6 +264,7 @@ class SessionImpact:
heartbeat_stale: bool heartbeat_stale: bool
is_requester: bool is_requester: bool
live: bool live: bool
connector: str | None = None
def as_dict(self) -> dict[str, Any]: def as_dict(self) -> dict[str, Any]:
return { return {
@@ -87,6 +277,7 @@ class SessionImpact:
"heartbeat_stale": self.heartbeat_stale, "heartbeat_stale": self.heartbeat_stale,
"is_requester": self.is_requester, "is_requester": self.is_requester,
"live": self.live, "live": self.live,
"connector": self.connector,
} }
@@ -105,6 +296,7 @@ class LeaseImpact:
disruptive: bool disruptive: bool
is_mutation: bool is_mutation: bool
is_critical_section: bool is_critical_section: bool
connector: str | None = None
def as_dict(self) -> dict[str, Any]: def as_dict(self) -> dict[str, Any]:
return { return {
@@ -119,6 +311,7 @@ class LeaseImpact:
"disruptive": self.disruptive, "disruptive": self.disruptive,
"is_mutation": self.is_mutation, "is_mutation": self.is_mutation,
"is_critical_section": self.is_critical_section, "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).""" """Impact preview DTO returned to the console / operator (#642/#652)."""
coordinator_version: str 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 evaluated_at: str
dry_run: bool dry_run: bool
restart_performed: bool restart_performed: bool
@@ -153,6 +353,13 @@ class RestartImpactReport:
def as_dict(self) -> dict[str, Any]: def as_dict(self) -> dict[str, Any]:
return { return {
"coordinator_version": self.coordinator_version, "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, "evaluated_at": self.evaluated_at,
"dry_run": self.dry_run, "dry_run": self.dry_run,
"restart_performed": self.restart_performed, "restart_performed": self.restart_performed,
@@ -206,6 +413,7 @@ def _classify_session(
requesting_session_id and session_id == requesting_session_id requesting_session_id and session_id == requesting_session_id
), ),
live=live, 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, disruptive=disruptive,
is_mutation=is_mutation, is_mutation=is_mutation,
is_critical_section=disruptive, 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, requesting_session_id: str | None = None,
dry_run: bool = True, dry_run: bool = True,
session_heartbeat_stale_seconds: int = DEFAULT_SESSION_HEARTBEAT_STALE_SECONDS, 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: ) -> RestartImpactReport:
"""Evaluate a proposed MCP restart and return an impact preview. """Evaluate a proposed MCP restart and return an impact preview.
@@ -301,6 +518,61 @@ def evaluate_restart_impact(
""" """
moment = now or _utc_now() moment = now or _utc_now()
reasons: list[str] = [] 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)) inventory_complete = bool(inventory.get("inventory_complete", False))
incomplete_reasons = [str(r) for r in (inventory.get("incomplete_reasons") or [])] 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] lease_impacts = [_classify_lease(l) for l in leases_raw]
# Only *other* live sessions and live leases constitute blast radius: a # Route impact through the selected class. Narrow classes never inherit a
# restart that would kill only the requesting session with no other work in # full-runtime drain merely because unrelated work exists.
# flight is safe. 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 = [ 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] disruptive_leases = [l for l in scoped_leases if l.disruptive]
critical_sections = [l for l in lease_impacts if l.is_critical_section] critical_sections = [l for l in scoped_leases if l.is_critical_section]
mutations = [l for l in lease_impacts if l.is_mutation] 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( 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 verdict = VERDICT_UNSAFE
allow_restart = False allow_restart = False
reasons.append( reasons.append(
@@ -381,7 +720,7 @@ def evaluate_restart_impact(
f"{len(critical_sections)} critical section(s) in flight " f"{len(critical_sections)} critical section(s) in flight "
"(active lease with a live owner)" "(active lease with a live owner)"
) )
if terminal_lock: if terminal_lock_in_scope:
reasons.append("active terminal (merge) lock present") reasons.append("active terminal (merge) lock present")
override_would_allow = bool(inventory_complete and disruptive) override_would_allow = bool(inventory_complete and disruptive)
@@ -411,6 +750,12 @@ def evaluate_restart_impact(
audit_record = { audit_record = {
"event": "restart_impact_evaluated", "event": "restart_impact_evaluated",
"coordinator_version": COORDINATOR_VERSION, "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(), "evaluated_at": moment.isoformat(),
"dry_run": dry_run, "dry_run": dry_run,
"operator_override": bool(operator_override), "operator_override": bool(operator_override),
@@ -424,6 +769,15 @@ def evaluate_restart_impact(
return RestartImpactReport( return RestartImpactReport(
coordinator_version=COORDINATOR_VERSION, 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(), evaluated_at=moment.isoformat(),
dry_run=dry_run, dry_run=dry_run,
restart_performed=False, restart_performed=False,
@@ -440,9 +794,11 @@ def evaluate_restart_impact(
affected_issues=affected_issues, affected_issues=affected_issues,
affected_prs=affected_prs, affected_prs=affected_prs,
mutations=mutations, mutations=mutations,
terminal_lock=dict(terminal_lock) terminal_lock=(
if isinstance(terminal_lock, Mapping) dict(terminal_lock_in_scope)
else terminal_lock, if isinstance(terminal_lock_in_scope, Mapping)
else terminal_lock_in_scope
),
ack_state=ack_state, ack_state=ack_state,
prior_recovery_attempts=prior_recovery_attempts, prior_recovery_attempts=prior_recovery_attempts,
counts=counts, counts=counts,
@@ -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()
+117
View File
@@ -105,3 +105,120 @@ def test_cross_links_do_not_embed_secrets():
text = _read(path) text = _read(path)
for marker in ("ghp_", "BEGIN PRIVATE KEY", "Authorization: Bearer"): for marker in ("ghp_", "BEGIN PRIVATE KEY", "Authorization: Bearer"):
assert marker not in text, f"{path} contains {marker!r}" 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}"
+232
View File
@@ -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,
}
+6
View File
@@ -444,6 +444,12 @@ class TestAuditEmission(unittest.TestCase):
) )
self.assertEqual(record["target"]["namespace"], NAMESPACE) self.assertEqual(record["target"]["namespace"], NAMESPACE)
self.assertEqual(record["target"]["mode"], "restart") 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["result"], console_audit.RESULT_ALLOWED)
self.assertEqual(record["actor"]["subject"], "[email protected]") self.assertEqual(record["actor"]["subject"], "[email protected]")
self.assertFalse(record["metadata"]["process_kill_executed"]) self.assertFalse(record["metadata"]["process_kill_executed"])
+19 -1
View File
@@ -38,6 +38,7 @@ from dataclasses import asdict, dataclass
from typing import Any from typing import Any
import mcp_namespace_health import mcp_namespace_health
import restart_coordinator
import runtime_recovery_guard import runtime_recovery_guard
from webui import console_audit, console_authz from webui import console_audit, console_authz
@@ -99,6 +100,14 @@ def _clean(value: Any) -> str:
return str(value or "").strip() return str(value or "").strip()
def restart_class_for_mode(mode: str) -> str:
"""Map the existing namespace controls onto the #663 class taxonomy."""
if _clean(mode) == MODE_RELOAD:
return restart_coordinator.RestartClass.CONFIGURATION_RELOAD.value
return restart_coordinator.RestartClass.ROLE_RUNTIME_RESTART.value
# --- Mutation ledger -------------------------------------------------------- # --- Mutation ledger --------------------------------------------------------
@@ -256,6 +265,7 @@ def build_restart_preview(
return { return {
"action_id": action_id, "action_id": action_id,
"restart_class": restart_class_for_mode(md),
"namespace": ns, "namespace": ns,
"mode": md, "mode": md,
"scope_valid": scope_error is None, "scope_valid": scope_error is None,
@@ -309,6 +319,7 @@ def assess_restart_request(
"reason_code": reason_code, "reason_code": reason_code,
"detail": detail, "detail": detail,
"action_id": action_id, "action_id": action_id,
"restart_class": restart_class_for_mode(md),
"namespace": ns, "namespace": ns,
"mode": md, "mode": md,
"preview": preview, "preview": preview,
@@ -393,6 +404,7 @@ def assess_restart_request(
"process." "process."
), ),
"action_id": action_id, "action_id": action_id,
"restart_class": restart_class_for_mode(md),
"namespace": ns, "namespace": ns,
"mode": md, "mode": md,
"preview": preview, "preview": preview,
@@ -441,7 +453,11 @@ def execute_restart(
else console_audit.RESULT_DENIED else console_audit.RESULT_DENIED
), ),
principal=principal, principal=principal,
target={"namespace": assessment["namespace"], "mode": assessment["mode"]}, target={
"namespace": assessment["namespace"],
"mode": assessment["mode"],
"restart_class": assessment["restart_class"],
},
reason_code=assessment["reason_code"], reason_code=assessment["reason_code"],
detail=assessment["detail"], detail=assessment["detail"],
request_id=request_id, request_id=request_id,
@@ -450,6 +466,7 @@ def execute_restart(
"gates_passed": assessment["gates_passed"], "gates_passed": assessment["gates_passed"],
"process_kill_executed": False, "process_kill_executed": False,
"post_restart_verification_required": True, "post_restart_verification_required": True,
"restart_class": assessment["restart_class"],
}, },
) )
@@ -463,6 +480,7 @@ def execute_restart(
"namespace": assessment["namespace"], "namespace": assessment["namespace"],
"mode": assessment["mode"], "mode": assessment["mode"],
"action_id": action_id, "action_id": action_id,
"restart_class": assessment["restart_class"],
"process_kill_executed": False, "process_kill_executed": False,
"host_hook": assessment["preview"]["restart_hook"], "host_hook": assessment["preview"]["restart_hook"],
"next_action": ( "next_action": (